Bug 105139 – Rationalize array profiling for out-of-bounds and hole cases

• Status: RESOLVED
• Resolution: FIXED
• Component: JavaScriptCore

[Filip Pizlo]

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.

[Filip Pizlo]

Created attachment 179668 [details]
the patch

[Filip Pizlo]

Comment on attachment 179668 [details]
the patch

Oops, forgot about the 32-bit code.

[Filip Pizlo]

Created attachment 179681 [details]
the patch

[Geoffrey Garen]

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]

(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]

Landed in http://trac.webkit.org/changeset/137937

[Geoffrey Garen]

> > 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.