On 12/05/2011 2:42 PM, Corinna Vinschen wrote:
On May 12 13:53, Ryan Johnson wrote:
On 12/05/2011 12:55 PM, Corinna Vinschen wrote:
    heap *heaps;
This is a misnomer now -- it's really a list of heap regions/blocks.
I don't think so.  The loop stores only the blocks which constitute
the original VirtualAlloc'ed memory regions.  They are not the real
heap blocks returned by Heap32First/Heap32Next.  These are filtered
out by checking for flags&   2 (**).
Sorry, I cut too much context out of the diff. That's
heap_info::heaps, which indeed refers to heap regions which we
identified by checking flags&2 (that's why it needs the heap_id
inside it now, where it didn't before) (++)
So you think something like heap_chunks is better?
Maybe. I actually only noticed because the code snippet you originally sent also used 'heaps' as an identifier and the two clashed when I pasted it in ;)

+               heap *h = (heap *) cmalloc (HEAP_FHANDLER, sizeof (heap));
+               *h = (heap) {heaps, hcnt, barray[bcnt].Address,
+                            barray[bcnt].Address + barray[bcnt].Size,
+                            harray->Heaps[hcnt].Flags};
+               heaps = h;
Given that the number of heap blocks is potentially large, I think
Not really.  See (**).  3 heaps ->   3 blocks, or only slightly more
if a growable heap got expanded.
See (++). When I point my essentially-identical version of the code
at emacs, it reports 8 heaps, each with 2-4 regions. The list
traversed by fill_on_match has ~20 entries.
Oh, ok.  From my POV 20 is not a large number.  Ordering might take
more time than scanning.  I don't think it's worth the effort.
You might be right. I just threw my test case at Firefox, and even that memory hog generates < 50 entries. I guess it does some sort of manual memory management as well, because windbg reports waaaay more allocated address space regions than that...

Besides, you've shown that we potentially need each heap block multiple times, so delete-after-use isn't a good idea.

Ryan

Reply via email to