1683746 - Minor bytecode optimizations for private fields

Status: RESOLVED FIXED

(Core :: JavaScript Engine, enhancement, P1)

Description

[Jason Orendorff [:jorendorff]]

A few things could be improved at the margin:

• We do a private brand check twice in the bytecode for obj.#priv++. Also affects compound assignment. But if the first brand check passes, the second will too.

• We emit ToPropertyKey in the bytecode for obj.#m(), but it's a no-op since #m is always a Symbol (the private name).

• We emit a CheckAliasedLexical each time we look up a private symbol, like the #m in obj.#m(), because it's implemented as a bogus lexical binding. But the binding can't be uninitialized; we initialize it as part of setting up the class, long before any code using the private name can actually run.

Comment 1

[Jason Orendorff [:jorendorff]]

Attachment: Bug 1683746 - Part 1: Eliminate redundant brand check in compound assignment to private field. r?mgaudet

Comment 2

[Jason Orendorff [:jorendorff]]

Attachment: Bug 1683746 - Part 2: Don't emit ToPropertyKey if the key is a private name. r?mgaudet

Depends on D100284

Comment 3

[Jason Orendorff [:jorendorff]]

Attachment: Bug 1683746 - Part 3: Don't emit CheckAliasedLexical when fetch a private name or method. r=mgaudet

Private fields are implemented, in part, with bogus bindings like #f or
#m.method, stored in the class Environment, which contain the private
symbols, methods, getters, and setters for the class. Since these are not real
bindings, the spec never says to check that they are initialized. They're
always initialized as part of class setup, before they could possibly be used.
The CheckAliasedLexical instruction is, therefore, a no-op. This patch
eliminates it.

Depends on D100285

Comment 4

[BugBot [:suhaib / :marco/ :calixte]]

There are some r+ patches which didn't land and no activity in this bug for 2 weeks.
:jorendorff, could you have a look please?
For more information, please visit auto_nag documentation.

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/063b2af126f1
Part 1: Eliminate redundant brand check in compound assignment to private field. r=mgaudet
https://hg.mozilla.org/integration/autoland/rev/e80fe7b8b46b
Part 2: Don't emit ToPropertyKey if the key is a private name. r=mgaudet
https://hg.mozilla.org/integration/autoland/rev/a64719684a50
Part 3: Don't emit CheckAliasedLexical when fetch a private name or method. r=mgaudet

Comment 6

[Alexandru Michis [:malexandru]]

Backed out 5 changesets (Bug 1683746, Bug 1688025) for causing hazard bustages in Scope.cpp

Backout link: https://hg.mozilla.org/integration/autoland/rev/5faded16fe407277598335d564a019dc8cfaa56c

Push with failures: https://treeherder.mozilla.org/jobs?repo=autoland&fromchange=38d9f357fcb523cd9abfb6eeb71910c9bb53bf50&searchStr=linux%2Cx64%2Cdebug%2Cspidermonkey%2Cbuilds%2Chazard-linux64-shell-haz%2Fdebug%2Ch&tochange=5faded16fe407277598335d564a019dc8cfaa56c&selectedTaskRun=IgwjTuA0SjqYUL18-h8hEw.0

Failure log: https://treeherder.mozilla.org/logviewer?job_id=327832728&repo=autoland&lineNumber=5408

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/0920a2ae2126
Part 1: Eliminate redundant brand check in compound assignment to private field. r=mgaudet
https://hg.mozilla.org/integration/autoland/rev/529954587d3f
Part 2: Don't emit ToPropertyKey if the key is a private name. r=mgaudet
https://hg.mozilla.org/integration/autoland/rev/26fa34063dec
Part 3: Don't emit CheckAliasedLexical when fetch a private name or method. r=mgaudet

Comment 8

[Bogdan Tara[:bogdan_tara | bogdant]]

https://hg.mozilla.org/mozilla-central/rev/0920a2ae2126
https://hg.mozilla.org/mozilla-central/rev/529954587d3f
https://hg.mozilla.org/mozilla-central/rev/26fa34063dec