issue330

translator memory usage

[Malte, 2012-04-16 08:32:33]

http://codereview.appspot.com/6030056/diff/1/src/translate/tools.py
File src/translate/tools.py (right):

http://codereview.appspot.com/6030056/diff/1/src/translate/tools.py#newcode75
src/translate/tools.py:75: # This will only work on Linux/Unix systems.
Should be "Linux" rather than "Linux/Unix" -- other common Unixes (in particular
Mac OS X) don't support this.

http://codereview.appspot.com/6030056/diff/1/src/translate/tools.py#newcode83
src/translate/tools.py:83: return -1
I think we should print the warning whenever we return -1, i.e. also if for some
reason we fail to find the VmPeak line.

except IOError:
    pass
print "..."
return -1

Also, I think returning None would be more pythonic.

http://codereview.appspot.com/6030056/diff/1/src/translate/translate.py
File src/translate/translate.py (right):

http://codereview.appspot.com/6030056/diff/1/src/translate/translate.py#newco...
src/translate/translate.py:564: print "Translator peak memory: %d KB" %
tools.get_peak_memory_in_kb()
Rather than printing -1 on error, should do something like

peak_memory = ...
if peak_memory is not None:
    print "Translator peak memory: ..."
else:
    print "Could not determine memory usage" (or whatever)

That is, I think it makes sense to move the failure message out of
get_peak_memory_in_kb.

(Another question is whether get_peak_memory_in_kb should return None on failure
or just raise an exception to be caught here.)

[jendrik, 2012-04-16 11:22:30]

Made proposed changes.

http://codereview.appspot.com/6030056/diff/1/src/translate/tools.py
File src/translate/tools.py (right):

http://codereview.appspot.com/6030056/diff/1/src/translate/tools.py#newcode75
src/translate/tools.py:75: # This will only work on Linux/Unix systems.
On 2012/04/16 08:32:34, Malte wrote:
> Should be "Linux" rather than "Linux/Unix" -- other common Unixes (in
particular
> Mac OS X) don't support this.

Done.

http://codereview.appspot.com/6030056/diff/1/src/translate/tools.py#newcode83
src/translate/tools.py:83: return -1
On 2012/04/16 08:32:34, Malte wrote:
> I think we should print the warning whenever we return -1, i.e. also if for
some
> reason we fail to find the VmPeak line.
> 
> except IOError:
>     pass
> print "..."
> return -1
> 
> Also, I think returning None would be more pythonic.

Done.

http://codereview.appspot.com/6030056/diff/1/src/translate/translate.py
File src/translate/translate.py (right):

http://codereview.appspot.com/6030056/diff/1/src/translate/translate.py#newco...
src/translate/translate.py:564: print "Translator peak memory: %d KB" %
tools.get_peak_memory_in_kb()
I chose to raise a Warning if the value cannot be determined.