Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

get_csig() of Value Node bloats memory #2785

Open
bdbaddog opened this issue Jan 2, 2018 · 0 comments
Open

get_csig() of Value Node bloats memory #2785

bdbaddog opened this issue Jan 2, 2018 · 0 comments

Comments

@bdbaddog
Copy link
Contributor

bdbaddog commented Jan 2, 2018

This issue was originally created at: 2011-08-29 13:39:30.
This issue was reported by: rbaumann4.
rbaumann4 said at 2011-08-29 13:39:30

I use value nodes to represent test executions. The tests depend on test programs, i.e. binaries built with env.Program(). I observed MemoryError when the number of tests increased. I debugged this to the fact that the Value nodes use the contents of their children as signature. In this case, this cosumes megabybtes of memory, because the complete executables are kept in memory.

I think it would make more sense to compute the signature of value nodes similar to Alias nodes: use get_csig() of the children instead of their contents and compute an md5 sum over all signatures + Value nodes own contents.

rbaumann4 said at 2011-08-29 13:45:09

Created an attachment (id=865)
Patch

bdbaddog said at 2011-08-31 19:26:25

This is a duplicate of 2488, but since you have a patch, I'll mark 2488 a duplicate of this bug.

bdbaddog said at 2011-08-31 22:50:37

One issue with your patch is that if the size of the Value node is < the size of the MD5 hash, it will end up increasing the memory footprint without value.

How about comparing to the length of the hash and MD5'ing it only if it's >= ?

What's the purpose of concatenating the children nodes info in the get_text_content() method?

+        childsigs = [n.get_csig() for n in self.children(None)]
+        return contents + ''.join(childsigs)

rbaumann4 said at 2011-09-03 13:00:55

The patch I provided is something I think I can live with. I don't claim that it is perfect. Sure, if the MD5 is longer than the content, then it makes sense to take the content instead. Conerning the children node concatenation: The original implementation concatenated the content of the children to compute the signature. That was causing the memory bloat. So I simply concatenated the signatures of the child nodes instead. If the get_text_content() method is not the right place, then it should not be called by get_csig() and the concatenation of the child signatures should be done somewhere else. Or was the question rather if it makes sense to take the child nodes into account at all? I would say so. As the comment in the code indicates, the value node is computed from the content of its children.

dirkbaechle said at 2014-05-18 04:11:34

*** Issue 2488 has been marked as a duplicate of this issue. ***

dirkbaechle said at 2014-05-18 04:12:44

reassigning issue

rbaumann4 attached scons.patch at 2011-08-29 13:45:09.

Patch

dirkbaechle said this issue is duplicated by #2488 at 2014-05-18 04:11:34.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants