If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

Optimize accesses to unboxed expandos in Ion

RESOLVED FIXED in Firefox 40

Status

()

Core
JavaScript Engine: JIT
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: bhackett, Assigned: bhackett)

Tracking

(Blocks: 1 bug)

Trunk
mozilla40
x86
Mac OS X
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox40 fixed)

Details

Attachments

(1 attachment)

(Assignee)

Description

3 years ago
Created attachment 8584869 [details] [diff] [review]
patch

Bug 1137180 (extensible unboxed objects) regressed octane-box2d, due to what seems to be a lack of inline paths for accessing unboxed expando objects.  The attached patch adds these, and adds back baseline caches for SETPROP/ADDPROP on unboxed expandos, the former of which is necessary for this patch and the latter which still seems like it should be in place.  At least a fair amount of this is simplification (handling lists of receivers in IonBuilder, for example) instead of more unboxed objects related code metastasis.

This seems to have fixed the unreproducible linux x64 dt3 intermittent failure I got with the earlier baseline caches patch, but now there are some windows only intermittent devtools failures on try.  So I guess I'll just land this one in pieces until I can narrow down the problem.
Attachment #8584869 - Flags: review?(jdemooij)
(Assignee)

Comment 1

3 years ago
(In reply to Brian Hackett (:bhackett) from comment #0)
> This seems to have fixed the unreproducible linux x64 dt3 intermittent
> failure I got with the earlier baseline caches patch, but now there are some
> windows only intermittent devtools failures on try.  So I guess I'll just
> land this one in pieces until I can narrow down the problem.

Actually, the orange in the try push was preexisting from another bug.  Here's the push FWIW:

https://treeherder.mozilla.org/#/jobs?repo=try&revision=f67644a69017
Comment on attachment 8584869 [details] [diff] [review]
patch

Review of attachment 8584869 [details] [diff] [review]:
-----------------------------------------------------------------

::: js/src/jit/BaselineIC.h
@@ +3926,5 @@
>          return offsetof(ReceiverGuard, group_);
>      }
>  
>      // Bits to munge into IC compiler keys when that IC has a ReceiverGuard.
>      // This uses at two bits for data.

Nit: at most?

::: js/src/jit/MIR.cpp
@@ +4482,3 @@
>          const Shape *shape = this->shape(i);
> +        if (!shape)
> +            continue;

This is ok because StoreFixedSlot/StoreSlot are never used for unboxed properties?
Attachment #8584869 - Flags: review?(jdemooij) → review+
(Assignee)

Comment 3

3 years ago
Yeah, StoreFixedSlot/StoreSlot are only used to store Values.

https://hg.mozilla.org/integration/mozilla-inbound/rev/b9da4b71accc
https://hg.mozilla.org/mozilla-central/rev/b9da4b71accc
Assignee: nobody → bhackett1024
Status: NEW → RESOLVED
Last Resolved: 3 years ago
status-firefox40: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla40
You need to log in before you can comment on or make changes to this bug.