Closed
Bug 1096539
Opened 10 years ago
Closed 10 years ago
Keep track of typed object reference properties in type information
Categories
(Core :: JavaScript Engine, defect)
Core
JavaScript Engine
Tracking
()
RESOLVED
FIXED
mozilla36
People
(Reporter: bhackett1024, Assigned: bhackett1024)
References
Details
Attachments
(2 files)
(deleted),
patch
|
nmatsakis
:
review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
nmatsakis
:
review+
|
Details | Diff | Splinter Review |
Currently type information only keeps track of properties and elements for native objects. For typed objects we should do this now. This is only necessary for object and value reference properties; scalar and string properties only contain specific types. The attached patch fixes this and adds IonMonkey support for optimizing property loads based on type information, avoiding type barriers.
Type information for typed objects isn't as good as for normal native objects, as typed objects always implicitly contain the initial values for typed object properties (null object pointers or undefined values) even if those initial values are immediately overwritten when the typed object is created. This mainly results in requiring null checks when reading from object fields of typed objects (as a Java compiler would do.) Eventually we should be able to do better here, though only when the typed object is always initialized in a particular way, such as new FooStruct({a:b, c:d}) (which, after escape analysis, won't allocate a temporary object when running in Ion.)
Between this and bug 109592, the generated code on the recently added simple-struct-typedobj test on awfy-assorted is within 1% in instruction count of what we generate for the comparable code written with normal objects. The typed object version still runs twice as slow though, which I need to investigate more.
Attachment #8520136 -
Flags: review?(nmatsakis)
Comment 1•10 years ago
|
||
Comment on attachment 8520136 [details] [diff] [review]
patch
Review of attachment 8520136 [details] [diff] [review]:
-----------------------------------------------------------------
Sorry for the delay on this. Bad week. Catching up now.
::: js/src/builtin/TypedObject.cpp
@@ +2916,2 @@
> {
> + // Undefined values are implicit for value references.
It took me a bit to figure out what this comment (and the similar ones below) meant, could you elaborate a bit? For example:
// Initial values of properties (undefined, in this case) are not reflected in the TI information, they are implicitly present.
::: js/src/jit/MIR.cpp
@@ +4391,5 @@
> if (property.couldBeConstant(constraints))
> return false;
> +
> + types::TypeSet *types = property.maybeTypes();
> + if (implicitType != MIRType_None) {
Can you add a comment here explaining when `implicitType` might not be None, and why it makes sense to do...whatever this code is doing? Maybe it's the time of night, but this totally goes over my head, even though I think I understand what it must logically be doing in order for this patch to make sense. (I'm presuming you've gotten it right, in any case.)
Attachment #8520136 -
Flags: review?(nmatsakis) → review+
Assignee | ||
Comment 2•10 years ago
|
||
Unfortunately, while this works in simple cases, due to some dumb bugs and oversights in the patch we still end up with barriers in unnecessary cases. This patch applies on top of the previous one, cleaning it up some and fixing these problems.
Assignee: nobody → bhackett1024
Attachment #8523572 -
Flags: review?(nmatsakis)
Updated•10 years ago
|
Attachment #8523572 -
Flags: review?(nmatsakis) → review+
Assignee | ||
Comment 3•10 years ago
|
||
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla36
You need to log in
before you can comment on or make changes to this bug.
Description
•