Closed
Bug 832329
Opened 12 years ago
Closed 12 years ago
Beef up definite properties analysis for Prototype library's create() method
Categories
(Core :: JavaScript Engine, defect)
Tracking
()
RESOLVED
FIXED
mozilla21
People
(Reporter: bhackett1024, Unassigned)
References
Details
Attachments
(1 file)
(deleted),
patch
|
jandem
:
review+
|
Details | Diff | Splinter Review |
The definite properties analysis used to determine which properties will definitely be possessed by an object created with 'new' doesn't pattern match sufficiently well to deal with the initialize.apply() call in Class.create(). With bug 638794 fixed this is all that's left to do to get definite properties in these objects, which gives a boost of about 23% to our v8-raytrace score (8166 -> 100044, x86/darwin).
Attachment #703914 -
Flags: review?(dvander)
Updated•12 years ago
|
Attachment #703914 -
Flags: review?(dvander) → review?(jdemooij)
Comment 1•12 years ago
|
||
Comment on attachment 703914 [details] [diff] [review]
patch
Review of attachment 703914 [details] [diff] [review]:
-----------------------------------------------------------------
Nice.
::: js/src/jsinfer.cpp
@@ +4997,5 @@
> + * could cause 'this' to escape.
> + *
> + * - The accessed property is either already a definite property or
> + * is not later added as one. Since the definite properties are
> + * added to the object at theh point of its creation, reading a
Nit: "the" typo
@@ +4998,5 @@
> + *
> + * - The accessed property is either already a definite property or
> + * is not later added as one. Since the definite properties are
> + * added to the object at theh point of its creation, reading a
> + * definite property before it is unassigned could incorrectly hit.
Nit: s/unassigned/assigned
@@ +5016,5 @@
> + Shape *shape = type->proto ? type->proto->nativeLookup(cx, id) : NULL;
> + if (shape && shape->hasSlot()) {
> + Value protov = type->proto->getSlot(shape->slot());
> + TypeSet *types = script->analysis()->bytecodeTypes(pc);
> + types->addType(cx, GetValueType(cx, protov));
This deserves a comment.
Attachment #703914 -
Flags: review?(jdemooij) → review+
Reporter | ||
Comment 2•12 years ago
|
||
Comment 3•12 years ago
|
||
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla21
You need to log in
before you can comment on or make changes to this bug.
Description
•