Closed Bug 1388354 Opened 2 years ago Closed 2 years ago

Optimize ToPropertyKey a bit

Categories

(Core :: JavaScript Engine, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla57
Tracking Status
firefox57 --- fixed

People

(Reporter: jandem, Assigned: jandem)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

Attached patch PatchSplinter Review
This patch adds a fast path for primitives to ToPropertyKey and moves the less-interesting object case to ToPropertyKeySlow. It eliminates some rooting overhead and is a small win on some SETELEM micro-benchmarks I tried (but it's noisy).
Attachment #8894870 - Flags: review?(andrebargull)
Comment on attachment 8894870 [details] [diff] [review]
Patch

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

I guess, similar to the other bugs, MOZ_ALWAYS_INLINE is also necessary here, because otherwise the compiler doesn't inline the function?
Attachment #8894870 - Flags: review?(andrebargull) → review+
(In reply to André Bargull from comment #1)
> I guess, similar to the other bugs, MOZ_ALWAYS_INLINE is also necessary
> here, because otherwise the compiler doesn't inline the function?

The compiler (at least GCC on Linux) did not inline it without my changes. I like to add MOZ_ALWAYS_INLINE to non-trivial but hot functions that I think should be inlined to ensure all compilers at least agree on this.
https://hg.mozilla.org/mozilla-central/rev/973ca5df0887
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla57
You need to log in before you can comment on or make changes to this bug.