482488 - -memstats periodically hanging on mac

Status: VERIFIED FIXED

(Tamarin Graveyard :: Garbage Collection (mmGC), defect, P2)

Description

[Brent Baker]

I am seeing periodic hangs when using -memstats and running the sunspider/string-unpack-code.as test.

I have reproduced the issue on 3 different 10.4 machines (intel and PPC), but have not been able to reproduce on a 10.5 machine.

The hang happens in both release and release_debugger builds but not in the debug builds.

Comment 1

[Erik Tierney]

Attachment: Fix memstats hang on mac

Memstats was hanging because the blocks were getting screwed up.  The code to merge blocks was wrong, and was merging to blocks together that were not contiguous.  Changed the code to make sure the two blocks were contiguous before merging them.  

Also added some ifdef'ed out debugging code to look for errors in blocks/regions.

Comment 2

[Tommy Reilly]

Comment on attachment 367282 [details] [diff] [review]
Fix memstats hang on mac

I don't think this is right, non-contiguous heap sections should be separated with a sentinel block.  This is old code that hasn't changed in awhile, something else changed recently that introduced this hang

Comment 3

[Erik Tierney]

Attachment: Add in sentinel when removing blocks

Change RemoveBlock to add in a sentinel since any block removed from the middle will mean the prev and next blocks are not contiguous.

Comment 4

[Erik Tierney]

pushed:
http://hg.mozilla.org/tamarin-redux/rev/de7b873a09a7

Comment 5

[Erik Tierney]

Attachment: Turn memstats fix back on, with fixes

This fixes the problems we were having with my previous fix.  Now it only inserts a sentinel if the block is removed from the middle of a contiguous region.  If the block is not contigous with either the previous or next block then there will already be a sentinel.  If the block is not contigous with both the previous and next block, we now remove a sentinel, since there would have been 2 (one on either side).  

Also patches up some bad math when we were redoing the region->blockIds and the prev/next pointers for the blocks that got moved.

Comment 6

[Tommy Reilly]

Comment on attachment 367784 [details] [diff] [review]
Turn memstats fix back on, with fixes

There's a redundant setting of need_sentinel to false but otherwise looks good

Comment 7

[Erik Tierney]

pushed:
http://hg.mozilla.org/tamarin-redux/rev/cbeb92c4f2aa

Comment 8

[Chris Peyer]

Resolved fixed engineering / work item that has been pushed.  Setting status to verified.