Bug 105139

Summary: Rationalize array profiling for out-of-bounds and hole cases
Product: WebKit Reporter: Filip Pizlo <fpizlo>
Component: JavaScriptCoreAssignee: Filip Pizlo <fpizlo>
Status: RESOLVED FIXED    
Severity: Normal CC: barraclough, ggaren, mark.lam, mhahnenberg, msaboff, oliver, sam
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Attachments:
Description Flags
the patch
none
the patch ggaren: review+

Filip Pizlo
Reported 2012-12-16 16:19:08 PST
Currently the out-of-bounds case is only detected via rare case profiling, which doesn't work if we had out-of-bounds in LLInt. Also we have a mixed reliance on exit profiling even though we don't necessarily need it, and we don't have enough ExitKinds to capture all of the cases, which makes looking at OSR exits via the profiler frustrating.
Attachments
the patch (31.05 KB, patch)
2012-12-16 16:20 PST, Filip Pizlo 		no flags
DetailsFormatted DiffDiff
the patch (36.52 KB, patch)
2012-12-16 18:59 PST, Filip Pizlo 		ggaren: review+
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Filip Pizlo
Comment 1 2012-12-16 16:20:48 PST
Created attachment 179668 [details] the patch
Filip Pizlo
Comment 2 2012-12-16 16:56:06 PST
Comment on attachment 179668 [details] the patch Oops, forgot about the 32-bit code.
Filip Pizlo
Comment 3 2012-12-16 18:59:21 PST
Created attachment 179681 [details] the patch
Geoffrey Garen
Comment 4 2012-12-16 21:40:56 PST
Comment on attachment 179681 [details] the patch View in context: https://bugs.webkit.org/attachment.cgi?id=179681&action=review r=me > Source/JavaScriptCore/bytecode/ArrayProfile.cpp:106 > + && !Heap::isMarked(m_expectedStructure)) Heap::isMarked() will be false if m_expectedStructure is newly allocated. (Not new here, but seems like a potential bug.)
Filip Pizlo
Comment 5 2012-12-16 23:14:14 PST
(In reply to comment #4) > (From update of attachment 179681 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=179681&action=review > > r=me > > > Source/JavaScriptCore/bytecode/ArrayProfile.cpp:106 > > + && !Heap::isMarked(m_expectedStructure)) > > Heap::isMarked() will be false if m_expectedStructure is newly allocated. (Not new here, but seems like a potential bug.) Note that this is only called when OperationInProgress == Collection. That will only happen when we're in the marking phase, at which point newly allocated objects should have Heap::isMarked(object) == true. Does that sound right?
Filip Pizlo
Comment 6 2012-12-17 13:40:11 PST
Landed in http://trac.webkit.org/changeset/137937
Geoffrey Garen
Comment 7 2012-12-17 16:42:11 PST
> > Heap::isMarked() will be false if m_expectedStructure is newly allocated. (Not new here, but seems like a potential bug.) > > Note that this is only called when OperationInProgress == Collection. That will only happen when we're in the marking phase, at which point newly allocated objects should have Heap::isMarked(object) == true. > > Does that sound right? Ah, yes.
Note You need to log in before you can comment on or make changes to this bug.