1781183 - Precalculate principal for AutoCheckRecursionLimit in frontend

Status: RESOLVED FIXED

(Core :: JavaScript Engine, task, P1)

Description

[Tooru Fujisawa [:arai]]

The principal calculation requires JSContext*, and also the calculation inside AutoCheckRecursionLimit is problematic in term of performance.

https://searchfox.org/mozilla-central/rev/23bf1890e07f780ba70e075bc8f46ffb85d1128c/js/public/friend/StackLimits.h#161-172

MOZ_ALWAYS_INLINE bool AutoCheckRecursionLimit::checkWithStackPointerDontReport(
    JSContext* cx, void* sp) const {
  // getStackLimitSlow(cx) is pretty slow because it has to do an uninlined
  // call to runningWithTrustedPrincipals to determine which stack limit to
  // use. To work around this, check the untrusted limit first to avoid the
  // overhead in most cases.
  uintptr_t untrustedLimit =
      getStackLimitHelper(cx, JS::StackForUntrustedScript, 0);
  if (MOZ_LIKELY(checkLimitImpl(untrustedLimit, sp))) {
    return true;
  }
  return checkLimitImpl(getStackLimitSlow(cx), sp);

Inside frontend, the principal is already known and it doesn't change during single compilation, and we should pre-calculate it and feed the stack limit directly.

Comment 1

[Tooru Fujisawa [:arai]]

Attachment: Bug 1781183 - Part 1: Add uintptr_t variant methods to AutoCheckRecursionLimit. r?bthrall!

Instead of using JSContext* to calculate the stack limit internally,
receive the stack limit as parameter.

Comment 2

[Tooru Fujisawa [:arai]]

Attachment: Bug 1781183 - Part 2: Add JSContext::stackLimitForCurrentPrincipal. r?bthrall!

Move the stack limit calculation around principal into JSContext method,
so that some consumer can pre-calculate it and feed it to
AutoCheckRecursionLimit.

Depends on D152772

Comment 3

[Tooru Fujisawa [:arai]]

Attachment: Bug 1781183 - Part 3: Add ParserBase::stackLimit_. r?bthrall!

Depends on D152773

Comment 4

[Tooru Fujisawa [:arai]]

Attachment: Bug 1781183 - Part 4: Simplify BytecodeEmitter constructor. r?bthrall!

parent parameter is used only internally.
Remove parent parameter from public methods.

This simplifies the stack limit parameter in the next patch.

Depends on D152774

Comment 5

[Tooru Fujisawa [:arai]]

Attachment: Bug 1781183 - Part 5: Add BytecodeEmitter::stackLimit. r?bthrall!

Depends on D152775

Comment 6

[Tooru Fujisawa [:arai]]

Attachment: Bug 1781183 - Part 6: Remove redundant field from FoldVisitor. r?bthrall!

FoldVisitor is subclass of RewritingParseNodeVisitor, which has
JSContext* field. and FoldVisitor doesn't have to have the same field.

Depends on D152776

Comment 7

[Tooru Fujisawa [:arai]]

Attachment: Bug 1781183 - Part 7: Pass stack limit to AutoCheckRecursionLimit. r?bthrall!

Depends on D152777

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/4e1f50a9d0d7
Part 1: Add uintptr_t variant methods to AutoCheckRecursionLimit. r=bthrall
https://hg.mozilla.org/integration/autoland/rev/17314cfdf984
Part 2: Add JSContext::stackLimitForCurrentPrincipal. r=bthrall
https://hg.mozilla.org/integration/autoland/rev/a6da48864831
Part 3: Add ParserBase::stackLimit_. r=bthrall
https://hg.mozilla.org/integration/autoland/rev/ad790c2acf09
Part 4: Simplify BytecodeEmitter constructor. r=bthrall
https://hg.mozilla.org/integration/autoland/rev/aff25780db95
Part 5: Add BytecodeEmitter::stackLimit. r=bthrall
https://hg.mozilla.org/integration/autoland/rev/240d1931ce20
Part 6: Remove redundant field from FoldVisitor. r=bthrall
https://hg.mozilla.org/integration/autoland/rev/c6adbe1e7195
Part 7: Pass stack limit to AutoCheckRecursionLimit. r=bthrall

Comment 9

[Sandor Molnar[:smolnar]]

https://hg.mozilla.org/mozilla-central/rev/4e1f50a9d0d7
https://hg.mozilla.org/mozilla-central/rev/17314cfdf984
https://hg.mozilla.org/mozilla-central/rev/a6da48864831
https://hg.mozilla.org/mozilla-central/rev/ad790c2acf09
https://hg.mozilla.org/mozilla-central/rev/aff25780db95
https://hg.mozilla.org/mozilla-central/rev/240d1931ce20
https://hg.mozilla.org/mozilla-central/rev/c6adbe1e7195