Closed
Bug 777643
Opened 12 years ago
Closed 12 years ago
JM: fix arguments.length jsop_getprop fast path
Categories
(Core :: JavaScript Engine, defect)
Core
JavaScript Engine
Tracking
()
RESOLVED
FIXED
mozilla17
People
(Reporter: luke, Assigned: luke)
References
Details
(Whiteboard: [js:t])
Attachments
(1 file)
2.78 KB,
patch
|
bhackett1024
:
review+
|
Details | Diff | Splinter Review |
Right now arguments.length will always go through stubs::GetProp, even when !script->needsArgsObj() since the types->isMagicArguments test is inside the top->mightBeType(JSVAL_TYPE_OBJECT) branch, which is going to be false for optimized arguments.
Assignee | ||
Comment 1•12 years ago
|
||
This seems to give back 5% on earley-boyer. This probably explains the regressions in bug 712714.
Comment 2•12 years ago
|
||
Comment on attachment 646326 [details] [diff] [review] fix Review of attachment 646326 [details] [diff] [review]: ----------------------------------------------------------------- ::: js/src/methodjit/Compiler.cpp @@ +4785,5 @@ > } > return true; > } > > + /* Handle lenth accesses of optimize 'arguments'. */ typos @@ +4788,5 @@ > > + /* Handle lenth accesses of optimize 'arguments'. */ > + if (name == cx->runtime->atomState.lengthAtom && > + cx->typeInferenceEnabled() && > + analysis->poppedTypes(PC, 0)->isMagicArguments(cx) && the poppedTypes call can just be 'top', right?
Attachment #646326 -
Flags: review?(bhackett1024) → review+
Assignee | ||
Comment 3•12 years ago
|
||
I assumed there was some subtle difference between the type info in a FrameEntry and in poppedTypes since the code right below this uses both 'top' and 'types'. Also, isMagicArguments is on the TypeSet.
Assignee | ||
Comment 4•12 years ago
|
||
(Are they equivalent? It isn't easy to see from the source.)
Comment 5•12 years ago
|
||
Oops, you're right, poppedTypes is better to use here as top is a FrameEntry rather than a TypeSet.
Assignee | ||
Comment 6•12 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/01a7a9d46117
Target Milestone: --- → mozilla17
Updated•12 years ago
|
Whiteboard: [js:t]
Comment 7•12 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/01a7a9d46117
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Comment 8•12 years ago
|
||
What regressed this bug? Please set as blocked by this bug. /be
You need to log in
before you can comment on or make changes to this bug.
Description
•