Closed Bug 1208850 Opened 4 years ago Closed 4 years ago

Inline functions exported to self-hosting global

Categories

(Core :: JavaScript Engine: JIT, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla44
Tracking Status
firefox42 --- unaffected
firefox43 + fixed
firefox44 + fixed

People

(Reporter: anba, Assigned: jandem)

Details

Attachments

(1 file)

intrinsic_functions in vm/SelfHosting.cpp should use JS_INLINABLE_FN instead of JS_FN for inlinable functions. 


Running the following function requires ~2s. After replacing JS_FN with JS_INLINABLE_FN for Math.abs, the function completes in 200ms. (Note: Math.abs is used in Number_isSafeInteger.)


function fn() {
  var numbers = [];
  for (var i = 0; i < 100000; ++i) {
    numbers[i] = (Math.random() * 100000)|0;
  }
  var c = 0;
  var start = Date.now();
  for (var i = 0; i < 1000; ++i) {
    for (var j = 0; j < 100000; ++j) {
      if (Number.isSafeInteger(numbers[j])) c++;
    }
  }
  var end = Date.now();
  assertEq(c, 1000 * 100000);
  return (end - start);
}
Heh, good catch.
Flags: needinfo?(jdemooij)
Attached patch PatchSplinter Review
Assignee: nobody → jdemooij
Status: NEW → ASSIGNED
Flags: needinfo?(jdemooij)
Attachment #8666670 - Flags: review?(till)
Comment on attachment 8666670 [details] [diff] [review]
Patch

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

Oh man, such a good catch! I didn't even know about JS_INLINABLE_FN.
Attachment #8666670 - Flags: review?(till) → review+
Comment on attachment 8666670 [details] [diff] [review]
Patch

Approval Request Comment
[Feature/regressing bug #]: Bug 1200809.
[User impact if declined]: Performance regressions.
[Describe test coverage new/current, TreeHerder]: On m-c for days.
[Risks and why]: Very low risk.
[String/UUID change made/needed]: None.
Attachment #8666670 - Flags: approval-mozilla-aurora?
https://hg.mozilla.org/mozilla-central/rev/bcca66a1c253
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla44
This fixed the regression that bug 1200809 had introduced on ss-fasta on AWFY.
Tracking since this is a recent regression.
Comment on attachment 8666670 [details] [diff] [review]
Patch

Fix for recent perf regression, did ok on m-c. Approved for aurora uplift.
Attachment #8666670 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
You need to log in before you can comment on or make changes to this bug.