print memory usage and a truncated version of the object in whos() #12791
Conversation
|
This looks really nice and does seem to give the right intuitive sense of memory usage. Does this "cover" all memory that the program can reach, or does memory in structures that could have cycles get skipped? |
probably not, although that's actually quite intentional since there's a huge ambiguity issue with that goal. so instead this is hugely conservative on the notion of ownership. basically, if it can be transferred away to another object without the parent object noticing, than it didn't "own" that memory. it also means that if a structure could have a cycle, then I don't want to try to sum all of the memory. i could give lots of examples, but consider for example a node in a linked list. while it's true that you could reach every other node in that list (and then all of their data and so on), when I am looking at a particular node, my intuition for memory consumption is most closely "how much memory does this node consume", not "how much valid memory can I manage address by following this pointer" (note, the CI errors look like they may be pre-existing bugs) |
|
That makes sense. It should just be documented when this does or doesn't comprehensively account for memory usage. It may be useful to have some option for reporting how much reachable memory is not accounted for by this reporting and probably some other stats about memory usage. |
…cblock to dominate the function
…them causing the whos test to fail
…st tests. the Base module should be clean though
|
travis failure appears to have been a random OOM event, considering the rest all passed, so I think this is ready to merge. |
|
we can tweak this over time if its not entirely optimal, but lets see how it goes for a bit on master |
|
as an aside, it's probably an interesting visualization question: even if you had all the information, i.e., a precise breakdown of reachable memory for every subset of roots, how would you display that ? |
|
i think the biggest problem is that you really only want to include memory with a clear parent -> child relationship, and reachability isn't really it. I wanted this number to be fairly stable against changes from distant parts of the system. although it can also be just as interesting to see how you can mislead the computation of this metric: |
print memory usage and a truncated version of the object in whos()
|
I think this might be a bit too arbitrary: |
|
yeah, functions are a bit tricky since the difference between a Function and a Closure isn't really represented correctly by the type hierarchy |
|
That's not the issue there. The problem is the difference between a struct, a tuple, and an array. The answers don't make any sense. Why should a tuple lead to deeper recursion for size-counting than an array? I think there are 2 reasonable approaches: (1) report the space used by only the toplevel object, or (2) recur and avoid duplicates with an ObjectIdDict. Double-counting is very misleading, and I think giving spurious wrong answers here is much worse than giving nothing. Trying to debug memory using this could waste many hours. |
|
Another suspicious smell here is how many methods |
this will almost always end up grossly exaggerating the memory usage because it will follow connections that are not logically parent -> child ownership links. I can avoid the double counting by building a small ObjectIdDict for the single level of recursion. I was waiting to see if that was worth the cost.
true, I actually suspect a lot of these should have been methods of |
That's silly. The current implementation doesn't address this at all, and also exaggerates memory usage. The key here is that you need a mental model of what this thing does to make it useful. "Follows all pointers without double-counting" I would argue is a simple and useful model. That model explains exactly why each node of a linked list would seem to use O(n) space. You can use this right away, for example to see how much more space a linked list uses compared to an array with the same data. Another way to think about it is that it should reflect the space it would take to |
I mostly disagree. Something like |
not a bad way to think about it, but pretty much worthless for implementing it
except that following all pointers would imply that you you also follow the data pointers I think the following patch would helps limit the recursion a bit better: diff --git a/base/interactiveutil.jl b/base/interactiveutil.jl
index 99126c0..8f2d76a 100644
--- a/base/interactiveutil.jl
+++ b/base/interactiveutil.jl
@@ -530,7 +530,7 @@ function summarysize(obj::Tuple, recurse::Bool)
if recurse
for val in obj
if val !== obj && !isbits(val)
# treat Tuple the same as any other iterable
- size += summarysize(val, recurse)::Int
+ size += summarysize(val, false)::Int
end
end
end
@@ -589,9 +589,11 @@ summarysize(obj::Set, recurse::Bool) =
function summarysize(obj::Function, recurse::Bool)
size::Int = sizeof(obj)
# don't unilaterally include `env` in the Function cost
- size += summarysize(obj.env, recurse)::Int
+ if recurse
+ size += summarysize(obj.env, true)::Int
+ end
if isdefined(obj, :code)
- size += summarysize(obj.code, recurse)::Int
+ size += summarysize(obj.code, true)::Int
end
return size
end |
Of course you would; that's part of the data. |
yes, but that's |
|
You're changing the subject. I'm responding to the "gross overcounting" objection. The reason I think it's not gross overcounting is that it's easy to understand what's going on. In contrast, I don't see how to make use of something that recurs 1 level. I don't think that helps you understand how much space something uses. |
|
|
Ok fair enough. |
I was pretty tempted to go with just printing |
|
But nobody will be able to intuit what these cheap hacks and "bit worse than" heuristics mean or where they come from. |
…nly include the summarysize of env in the count when at the toplevel ref #12791
that was in reference to this comment from above:
I do not think there is any generic algorithm that can tell you that. So far, the options seem to be:
|
|
I agree there isn't a perfect algorithm, I just want one simple enough to reason about. I think you can draw many useful conclusions from the I think special cases for certain objects like Module are justifiable. Maybe there could be a default list of types not to recur into, like |
|
Does this have any function to call to get the results as some sort of collection, rather than outputting it? |
…nly include the summarysize of env in the count when at the toplevel ref #12791
|
I think this may have started causing a segfault in the misc test - http://buildbot.e.ip.saba.us:8010/builders/build_ubuntu14.04-x64/builds/2310/steps/shell_2/logs/stdio |
| function summarysize(obj::Array, recurse::Bool) | ||
| size::Int = sizeof(obj) | ||
| if recurse && !isbits(eltype(obj)) | ||
| for i in 1:length(obj) |
There was a problem hiding this comment.
it's a plain array, so it doesn't need to be fancy
There was a problem hiding this comment.
Isn't eachindex just as fast as 1:length(obj)? Seems if so, be consistent and always use eachindex (or renamed to indexes, as discussed elsewhere?)
this test can be sensitive to invalid objects that get left behind by past tests |
|
Any way to make it more robust? Otherwise I have a bad feeling we'll start seeing this intermittently on CI, and now's not the time to be introducing any more of those... |
|
figure out what other test(s) are leaving behind invalid objects (this one appears to be an array for which calling length is an infinite recursion) |
|
This is another argument for why |
|
Actually, wasn't there supposed to be a feature freeze a week ago or so? Now we have to flail around with new intermittent CI segfaults, for yet another change that wasn't slated for 0.4. Great. |
|
let's please not conflate what seems likely to be increased test coverage increasing the likelihood of something causing a failure with that of an unrelated metric.
it was on the planning list ( |
|
It was moved to 0.4.x quite a while ago. The point is this is not a good time to disrupt the tests, even though such disruption is ultimately good since it exposes more issues to fix. This is why feature freezes exist, though it looks like we didn't officially announce one which is too bad. |
|
Another way to look at the algorithm here is that it basically gives If you want to play with |
|
The failure is 100% repeatable via |
this avoids the issues and complications of the previous attempts at this by explicitly defining minimally-recursive byte ownership relationships, rather than recursing over all fields and guessing too much. the general heuristic here is that if something could (in theory) form a recursive tree, then it doesn't exclusively "own" that memory, so it doesn't need to get counted. Only when an object is effectively just an indirection (by construction), did I add some rules to try to incorporate more of the fields. In practice, I think this makes the results pretty much as good as they possibly can be.
example output: