Closed Bug 1128644 Opened 7 years ago Closed 7 years ago

Use common offsets for unboxed layouts which are prefixes of each other

Categories

(Core :: JavaScript Engine, defect)

x86
macOS
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla38
Tracking Status
firefox38 --- fixed

People

(Reporter: bhackett1024, Assigned: bhackett1024)

References

Details

Attachments

(1 file)

Attached patch patchSplinter Review
If one object has a constructor like:

function Foo() {
  this.a = ...;
  this.b = ...;
  this.c = ...;
}

And another object has a constructor like:

function Bar() {
  this.b = ...;
  this.a = ...;
}

If both Foo and Bar are unboxed, we have control over the offsets given to each of their properties.  It would be nice if they used the same offsets for their properties that overlap, as if access sites are used with both Foo and Bar objects this would allow us to avoid having to branch on the type of the object.

The attached patch adds a simple variant of this optimization for unboxed objects.  When we make a new unboxed layout, we look at all the other unboxed layouts in the compartment (hopefully there won't be that many) to look for one which is a subset of the one we are creating.  If we find such a layout we use its offsets for the properties common to the two layouts.

This wins about 5% for me on octane-deltablue, when using unboxed objects.
Attachment #8558059 - Flags: review?(jdemooij)
Oh, after this patch deltablue is <5% faster with unboxed objects on x86, but about 20% faster on x64.
(In reply to Brian Hackett (:bhackett) from comment #0)
> When we make a new unboxed layout, we look at all the other
> unboxed layouts in the compartment (hopefully there won't be that many) to
> look for one which is a subset of the one we are creating.

If we did see this become a problem (say in a compiled C# codebase that has thousands of classes) then presumably we could do something better than linear search (e.g., by maintaining the layouts in a trie by field/type).
Attachment #8558059 - Flags: review?(jdemooij) → review+
https://hg.mozilla.org/mozilla-central/rev/358cc96cf83c
Assignee: nobody → bhackett1024
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla38
You need to log in before you can comment on or make changes to this bug.