port memory usage tests to windows #54

New issue

Open

opened 2007-05-25 03:49:55 +00:00 by zooko · 11 comments

zooko commented

2007-05-25 03:49:55 +00:00

Make the following also work on Windows: Measure the memory usage in different operations (upload, download, sustained server operation, ...) and graph it with a munin plugin. (Issue #29)

zooko added the

labels 2007-05-25 03:49:55 +00:00

warner was assigned by zooko

2007-05-25 03:49:55 +00:00

zooko commented

2007-05-28 23:14:33 +00:00

Author

Attachment thread628262.html (57027 bytes) added

tips and tricks for measuring CPU and RAM on Windows

**Attachment** thread628262.html (57027 bytes) added tips and tricks for measuring CPU and RAM on Windows

thread628262.html

56 KiB

zooko commented

2007-05-30 16:57:40 +00:00

Author

See #29 for the general memory-usage tests and fixing the memory-usage problems.

warner was unassigned by zooko

2007-05-30 16:57:40 +00:00

zooko self-assigned this 2007-05-30 16:57:40 +00:00

zooko changed title from ~~memory usage tests~~ to port memory usage tests to windows

2007-05-30 16:57:40 +00:00

warner added

c/code

and removed

c/dev-infrastructure

labels 2007-07-14 06:57:57 +00:00

warner removed the

c/code

label 2007-08-14 18:53:54 +00:00

zooko added

v/0.6.0

and removed

v/0.2.0

labels 2007-09-25 04:19:30 +00:00

zooko added this to the 0.6.1 milestone 2007-09-25 04:19:30 +00:00

zooko commented

2007-10-01 19:36:00 +00:00

Author

bumping this issue out of the v0.6.1 milestone because there are too many other more urgent things for this milestone

zooko modified the milestone from 0.6.1 to undecided

2007-10-01 19:36:00 +00:00

zooko commented

2009-05-04 17:47:01 +00:00

Author

If you love this ticket (#54), then you might like tickets #227 (our automated memory measurements might be measuring the wrong thing), #419 (pycryptopp uses up too much RAM), #478 (add memory-usage to stats-provider numbers), and #97 (reducing memory footprint in share reception).

daira added the

c/code

label 2009-12-04 05:14:40 +00:00

Grumpf commented

2009-12-20 16:10:06 +00:00

Owner

Attachment memcheck-win32.diff (4842 bytes) added

**Attachment** memcheck-win32.diff (4842 bytes) added

memcheck-win32.diff

4.7 KiB

Grumpf commented

2009-12-20 16:21:35 +00:00

Owner

Here is a patch proposal. It also makes "make check-memory" works under MinGW, with or without installed Twisted.

The "VmData" value doesn't seems to have any equivalence in GetProcessMemoryInfo() nor any use in current code, so might be stripped out.

Here is a patch proposal. It also makes "make check-memory" works under MinGW, with or without installed Twisted. The "VmData" value doesn't seems to have any equivalence in [GetProcessMemoryInfo](wiki/GetProcessMemoryInfo)() nor any use in current code, so might be stripped out.

daira commented

2009-12-21 02:12:44 +00:00

Comments on the patch:

on non-Windows a failure of the code in the try: block will cause the values to be left at zero, but on Windows a failure will propagate the exception. Make it consistent.
the duplication of code in [startstop_node.py]source:src/allmydata/scripts/startstop_node.py and [check_memory.py]source:src/allmydata/test/check_memory.py is undesirable -- move it to pyutil, perhaps.

Comments on the patch: * on non-Windows a failure of the code in the `try:` block will cause the values to be left at zero, but on Windows a failure will propagate the exception. Make it consistent. * the duplication of code in [startstop_node.py]source:src/allmydata/scripts/startstop_node.py and [check_memory.py]source:src/allmydata/test/check_memory.py is undesirable -- move it to pyutil, perhaps.

daira commented

2009-12-21 02:19:53 +00:00

Replying to davidsarah:

the duplication of code in [startstop_node.py]source:src/allmydata/scripts/startstop_node.py and [check_memory.py]source:src/allmydata/test/check_memory.py is undesirable -- move it to pyutil, perhaps.

Seems like a suitable place would be find_exe.find_twistd.

Replying to [davidsarah](/tahoe-lafs/trac/issues/54#issuecomment-361036): > * the duplication of code in [startstop_node.py]source:src/allmydata/scripts/startstop_node.py and [check_memory.py]source:src/allmydata/test/check_memory.py is undesirable -- move it to pyutil, perhaps. Seems like a suitable place would be `find_exe.find_twistd`.

zooko commented

2009-12-27 22:40:59 +00:00

Author

Grumpf: thank you for the patch!

So let's see, since David-Sarah's review indicated that this patch isn't ready to be committed then we should unset the reviewed keyword, right? By the way, http://allmydata.org/trac/pyutil/browser/pyutil/pyutil/memutil.py would be an ideal place for this functionality to live, eventually, but this doesn't directly help Tahoe-LAFS until and unless #47 (use pyutil as a separate package and contribute src/allmydata/util/* into pyutil) is closed.

Okay unsetting reviewed and assigning it to Grumpf to write a patch that fixes the issues raised by David-Sarah. Grumpf: I think David-Sarah is right that using source:src/allmydata/util/find_exe.py is a good way to do the "find twistd" behavior.

Grumpf: thank you for the patch! So let's see, since David-Sarah's review indicated that this patch isn't ready to be committed then we should unset the `reviewed` keyword, right? By the way, <http://allmydata.org/trac/pyutil/browser/pyutil/pyutil/memutil.py> would be an ideal place for this functionality to live, eventually, but this doesn't directly help Tahoe-LAFS until and unless #47 (use pyutil as a separate package and contribute src/allmydata/util/* into pyutil) is closed. Okay unsetting `reviewed` and assigning it to Grumpf to write a patch that fixes the issues raised by David-Sarah. Grumpf: I think David-Sarah is right that using source:src/allmydata/util/find_exe.py is a good way to do the "find twistd" behavior.

zooko commented

2009-12-27 22:49:43 +00:00

Author

Oh sorry, I didn't read your patch and see that you were already using find_exe. I guess the code duplication that David-Sarah was talking about was the part that comes after find_exe about looking for an executable that was installed in ./support relative to the basedir.

It is the code in source:src/allmydata/control.py which could potentially be merged with the comparable code in http://allmydata.org/trac/pyutil/browser/pyutil/pyutil/memutil.py .

Oh sorry, I didn't read your patch and see that you were already using `find_exe`. I guess the code duplication that David-Sarah was talking about was the part that comes after `find_exe` about looking for an executable that was installed in `./support` relative to the basedir. It is the code in source:src/allmydata/control.py which could potentially be merged with the comparable code in <http://allmydata.org/trac/pyutil/browser/pyutil/pyutil/memutil.py> .

zooko commented

2010-01-10 23:42:44 +00:00

Author

Grumpf: if you are going to work on this patch then please "accept" this ticket (using the control panel at the bottom of this page). If you don't then anyone else should feel free to update the patch to fix the details noted by David-Sarah's review.