Last Comment Bug 687642 - Change JSOP_GETELEM to use the narrower, split-by-property-type APIs
: Change JSOP_GETELEM to use the narrower, split-by-property-type APIs
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: JavaScript Engine (show other bugs)
: unspecified
: All All
: -- normal (vote)
: mozilla10
Assigned To: Jeff Walden [:Waldo] (remove +bmo to email)
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2011-09-19 14:11 PDT by Jeff Walden [:Waldo] (remove +bmo to email)
Modified: 2011-10-06 03:48 PDT (History)
3 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
1 - isIndex, not isElement (4.39 KB, patch)
2011-09-19 14:11 PDT, Jeff Walden [:Waldo] (remove +bmo to email)
dvander: review+
Details | Diff | Splinter Review
2 - Inject the name/element/special split into JSOP_GETELEM (11.71 KB, patch)
2011-09-19 14:13 PDT, Jeff Walden [:Waldo] (remove +bmo to email)
no flags Details | Diff | Splinter Review
2 - Unbitrotted (12.63 KB, patch)
2011-09-22 15:49 PDT, Jeff Walden [:Waldo] (remove +bmo to email)
dvander: review+
Details | Diff | Splinter Review

Description Jeff Walden [:Waldo] (remove +bmo to email) 2011-09-19 14:11:56 PDT
Created attachment 561014 [details] [diff] [review]
1 - isIndex, not isElement

This will give a pretty good idea how opcodes will be implemented using the property/index/special splitup.  It's a fairly easy first step that should be digestible on its own.

First step: PropertyName::isElement turns out to have been misnamed, should have been PropertyName::isIndex, so rename that.  Second step: rewrite the JSOP_GETELEM implementations.  This doesn't change any semantics, so I don't *think* I need to update the tracer, and I don't *think* I need to update PICs at all for it.  (Although perhaps there might be ways they could be updated to take advantage of this.  But that probably is better off waiting until the JSOp bytecodes are updated to be optimized for this split, since they aren't currently.)

These patches only apply atop bug 687621's patches (plus one other, but I think that third patch doesn't affect these), in case you decide to try them out.
Comment 1 Jeff Walden [:Waldo] (remove +bmo to email) 2011-09-19 14:13:02 PDT
Created attachment 561015 [details] [diff] [review]
2 - Inject the name/element/special split into JSOP_GETELEM
Comment 2 Jeff Walden [:Waldo] (remove +bmo to email) 2011-09-22 15:49:31 PDT
Created attachment 561914 [details] [diff] [review]
2 - Unbitrotted
Comment 3 Jeff Walden [:Waldo] (remove +bmo to email) 2011-09-22 16:03:53 PDT
https://hg.mozilla.org/integration/mozilla-inbound/rev/2f1a5172cced

This is only the first patch here -- second still needs to land, so merge-monkeys shouldn't mark this as fixed yet.  :-)
Comment 4 Ed Morley [:emorley] 2011-09-23 04:50:54 PDT
(In reply to Jeff Walden (remove +bmo to email) from comment #3)
> This is only the first patch here -- second still needs to land, so
> merge-monkeys shouldn't mark this as fixed yet.  :-)

Thanks ;-)

Part 1: https://hg.mozilla.org/mozilla-central/rev/2f1a5172cced
Comment 5 Ed Morley [:emorley] 2011-10-06 03:48:45 PDT
Part 2: https://hg.mozilla.org/mozilla-central/rev/0c2d5c359b0c

Note You need to log in before you can comment on or make changes to this bug.