Bug 105139 - Rationalize array profiling for out-of-bounds and hole cases
Summary: Rationalize array profiling for out-of-bounds and hole cases
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:
 
Reported: 2012-12-16 16:19 PST by Filip Pizlo
Modified: 2012-12-17 16:42 PST (History)
7 users (show)

See Also:


Attachments
the patch (31.05 KB, patch)
2012-12-16 16:20 PST, Filip Pizlo
no flags Details | Formatted Diff | Diff
the patch (36.52 KB, patch)
2012-12-16 18:59 PST, Filip Pizlo
ggaren: 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 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.
Comment 1 Filip Pizlo 2012-12-16 16:20:48 PST
Created attachment 179668 [details]
the patch
Comment 2 Filip Pizlo 2012-12-16 16:56:06 PST
Comment on attachment 179668 [details]
the patch

Oops, forgot about the 32-bit code.
Comment 3 Filip Pizlo 2012-12-16 18:59:21 PST
Created attachment 179681 [details]
the patch
Comment 4 Geoffrey Garen 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.)
Comment 5 Filip Pizlo 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?
Comment 6 Filip Pizlo 2012-12-17 13:40:11 PST
Landed in http://trac.webkit.org/changeset/137937
Comment 7 Geoffrey Garen 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.