src/python/m5/stats/__init__.py (Diff revision 1)

This is really strange. You first added the 'name' argument in a separate patch, and now you remove it here.

Show all issues

I understand the desire to avoid extra getattr lookups, but the way you're going about it seems rather confusing (and I believe could be done just as efficiently in a more clear manner).  Also, it's hard for me to suggest improvements because the optimizations are cluttered with extra code and spread across diffs.

src/python/m5/stats/__init__.py (Diff revision 1)

This seems like a bugfix that belongs in a different patch

Show all issues

src/python/m5/stats/sql.py (Diff revision 1)

This seems like it belongs in a different patch.

Show all issues

src/python/m5/stats/sql.py (Diff revision 1)

seems to me that this sort of thing belongs in the stat object, not here. Like a stat.getValue function.  I assume that the info is not available, so you can't use __getattr__ and it must be a function.  That's fine, but this seems like the wrong place to do this.

Show all issues

src/python/m5/stats/sql.py (Diff revision 1)

This seems like a pretty bad place to put this code.  Why does statContainer calculate the value instead of the stat itself?  Is it because the container has the info?  If so, it seems like you should have the container calculate the value instead of doing it here.

Show all issues

src/python/m5/stats/sql.py (Diff revision 1)

Seems like you should be looping over the values and recording if they're all zero, and if there are any NaN values instead of building an intermediate list (which is comparatively expensive).  You can also exit early.

hasNonNan = False
hasNonZero = False
for x in theValue:
  if not isnan(x):
    hasNonNan = True
    if hasNonZero:
      break
  if x:
    hasNonZero = True
    if hasNonNan:
      break

Show all issues