Closed Bug 1353679 Opened 4 years ago Closed 4 years ago

Optimize the Object case in obj_toString

Categories

(Core :: JavaScript Engine, enhancement)

enhancement
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla55
Tracking Status
firefox55 --- fixed

People

(Reporter: jandem, Assigned: jandem)

Details

Attachments

(2 files)

Attached patch PatchSplinter Review
This is by far the most common case when we get to the non-standard path, so it's worth checking for it.
Attachment #8854777 - Flags: review?(evilpies)
Comment on attachment 8854777 [details] [diff] [review]
Patch

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

Nice. Maybe we should atomize all the StringBuffer results.
Attachment #8854777 - Flags: review?(evilpies) → review+
This atomizes the result of toString. A microbenchmark is about 20% slower, but this will improve other parts of the system (GC, caches) so it seems worth it.
Attachment #8854867 - Flags: review?(evilpies)
Comment on attachment 8854867 [details] [diff] [review]
Part 2 - Atomize toString result

Hard to judge what the best decision here would be, but argument about improving the rest of the VM seems convincing. Although I haven't actually investigated this, I think it's likely a lot of code would do something toString().slice(8, -1) to extract just the actual tag.
Attachment #8854867 - Flags: review?(evilpies) → review+
Pushed by jandemooij@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/ed9386534ece
part 1 - Optimize the Object case in Object.prototype.toString because it's very common. r=evilpie
https://hg.mozilla.org/integration/mozilla-inbound/rev/23bb7ee563d9
part 2 - Atomize strings returned from Object.prototype.toString to avoid allocating duplicate strings. r=evilpie
https://hg.mozilla.org/mozilla-central/rev/ed9386534ece
https://hg.mozilla.org/mozilla-central/rev/23bb7ee563d9
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
You need to log in before you can comment on or make changes to this bug.