Bug 127463 - 3x splay regression in FTL with experimental coverage that is entirely due to Bartlett weirdness
Summary: 3x splay regression in FTL with experimental coverage that is entirely due to...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Filip Pizlo
URL:
Keywords:
Depends on:
Blocks: 112840
  Show dependency treegraph
 
Reported: 2014-01-22 22:09 PST by Filip Pizlo
Modified: 2014-01-23 17:14 PST (History)
10 users (show)

See Also:


Attachments
the patch (7.38 KB, patch)
2014-01-23 15:40 PST, Filip Pizlo
mhahnenberg: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Filip Pizlo 2014-01-22 22:09:11 PST
Hopefully, I'll find the right place to zero the stack or do other stuff just like all of the other times.
Comment 1 Oliver Hunt 2014-01-22 22:10:51 PST
O_o
Comment 2 Filip Pizlo 2014-01-22 22:16:24 PST
(In reply to comment #1)
> O_o

Dude, it's hilarious.  We get one of these every few months.

Splay trees imply essentially always inserting at the root and then having the root point at everything else.  Broadly speaking any node that was ever a root will continue to indefinitely have a transitive reference to every node that had been part of the tree at that time.

So, if you have a benchmark, like Splay, and at some point some pointer to the tree gets stuck on the stack and then you add and remove a bunch of things to the tree, then you end up doubling your heap size.  This is because that one stuck pointer will refer to all of the nodes that were the "old" tree at the time it got stuck, even if all of the nodes in that old tree get removed from the actual current tree.

But wait, there's more.  Because of how removal happens, the stuck pointer will be likely to transitively refer to every node that had ever been part of the tree since when the pointer got stuck until the present time.  So, in V8v7/splay, after a pointer gets stuck, you cease to be able to collect any garbage.

Interestingly, this almost never happens in-browser and only happens with the way I run splay in my harness.  So, it's not all that alarming.

In all cases when this happens, it's because a pointer gets stuck due to a goof in either OSR entry or in the GC's stack scan.  I'm guessing that this bug is due to FTL OSR entry.
Comment 3 Filip Pizlo 2014-01-22 22:34:36 PST
It looks like it requires that both splay_() and remove() get compiled with the FTL.  I can see the regression if just those two get compiled.  If I remove either one of them then the regression disappears.
Comment 4 Filip Pizlo 2014-01-23 14:45:59 PST
Ha!  This is entirely due to some sloppiness with scratch buffers.  I think that we claim to be using one at some point, then we dump a bunch of pointers into it, and then we never "free" it - so the GC keeps rescanning it ad infinitum.
Comment 5 Filip Pizlo 2014-01-23 15:40:38 PST
Created attachment 222033 [details]
the patch
Comment 6 Mark Hahnenberg 2014-01-23 15:42:41 PST
Comment on attachment 222033 [details]
the patch

r=me
Comment 7 Filip Pizlo 2014-01-23 17:14:19 PST
Landed in http://trac.webkit.org/changeset/162666