use a non-stack-based algorithm to compute summarysize #20161
Conversation
|
|
||
| Compute the amount of memory used by all unique objects reachable from the argument. | ||
| Keyword argument `exclude` specifies the types of objects to exclude from the traversal. | ||
| Keyword argument `chargeall` specifies the types of objects to always charge the size of all of their fields, even if those fields would normally be excluded. |
There was a problem hiding this comment.
Might be nice to provide keyword arguments as a bulleted list. Makes it a little easier to read. It would also be good to add line breaks to maintain the line length guidelines.
There was a problem hiding this comment.
Cf. reference example in the manual (bullet 4).
| Keyword argument `exclude` specifies the types of objects to exclude from the traversal. | ||
| Keyword argument `chargeall` specifies the types of objects to always charge the size of all of their fields, even if those fields would normally be excluded. | ||
| """ | ||
| function summarysize(obj::ANY; exclude::ANY = Union{DataType, TypeName}, chargeall::ANY = Union{TypeMapEntry, Method, Core.MethodInstance}) |
There was a problem hiding this comment.
Same comment regarding line lengths. You could just align exclude and chargeall beneath obj; that style is used elsewhere.
|
Is there a way to test that #19707 is fixed so that we can track it going forward? |
| end | ||
| if isa(obj, UnionAll) | ||
| # black-list of items that don't have a Core.sizeof | ||
| return 2 * sizeof(Int) |
There was a problem hiding this comment.
the old computation for UnionAll also added summarysize(obj.body, seen, excl) + summarysize(obj.var, seen, excl), is this new version undercounting?
There was a problem hiding this comment.
the fields are handle elsewhere now. (unrelated, but the old version was also over-counting the Core.sizeof)
| val = getfield(x, i) | ||
| end | ||
| end | ||
| if nfields(x) > i |
in addition to not using stack space, this doesn't need as many special cases
vtjnash
force-pushed
the
jn/summarysize-no-stack
branch
from
a62e179 to
8cd9d22
Compare
nothing too fancy here, just swapping the existing stack-based DFS algorithm for a heap-based one. this'll prevent this method from ever causing stack overflow, which will fix #19707.