Jean-Yves Avenard [:jya] Reporter
	Description • 6 years ago

From reddit:
https://www.reddit.com/r/firefox/comments/9wbgg5/why_are_arrow_functions_in_firefoxv65_nightly_6x/?utm_source=reddit-android

	mattmarcus58
	Comment 1 • 6 years ago

Link to reproduce: https://jsperf.com/function-vs-arrow-function
I can attest that I experienced similar slowdowns (~70%).

	khagaroth
	Comment 2 • 6 years ago

Weird one:
Normal profile - 83% slower
Addons disabled - as above
New clean profile - as above
Safe mode - arrow functions are 1% faster

	Tom S [:evilpie]
	Comment 3 • 6 years ago

Running this in the shell I see the exact same performance for both arrow and normal functions. Looking at the JIT spew, we compile away everything inside the loop. So this is probably some bad interaction with jsperf (again)

	Kannan Vijayan [:djvj]
	Comment 4 • 6 years ago

I can repro that on nightly, the results from jspref show arrows as slower than simple functions.  This is really bizarre - as evilpie noted we should be handling these and we do seem to be.

This is on nightly (65) Linux x86-64, with add-ons: uBlock, multi-account containers, gecko profiler, and temporary containers.

Tom, do you have a sense for the kinds of "weird jsperf interactions" that led to differential results from shell execution?

	André Bargull [:anba] Assignee
	Comment 5 • 6 years ago

MFunctionEnvironment::foldsTo [1] doesn't handle MLambdaArrow, which prevents moving the FunctionEnvironment out of the loop. The following change fixes the performance issue for me, but I'm not 100% sure why MLambdaArrow wasn't handled there (Did we simply forget to handle it or is it actually not allowed to handle MLambdaArrow for MFunctionEnvironment::foldsTo?)

---
MDefinition* MFunctionEnvironment::foldsTo(TempAllocator& alloc) {
  if (input()->isLambda()) {
    return input()->toLambda()->environmentChain();
  }
  if (input()->isLambdaArrow()) {
    return input()->toLambdaArrow()->environmentChain();
  }
  return this;
}
---

[1] https://searchfox.org/mozilla-central/rev/3564dfde3b125878dc5a04fe92629fc5195942df/js/src/jit/MIR.cpp#5086-5092

	André Bargull [:anba] Assignee
	Comment 6 • 6 years ago

The equivalent of bug 1069260, but this time for MLambdaArrow instead of MLambda.

Do you know if there's anything special about arrow-functions which prevents handling them in MFunctionEnvironment::foldsTo?

	Nicolas B. Pierron [:nbp]
	Comment 7 • 6 years ago

Comment on attachment 9029581 [details] [diff] [review]
bug1506524.patch

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

I think I might just have forgotten the case when I implemented it.
Thanks for catching and fixing this issue.

	André Bargull [:anba] Assignee
	Comment 8 • 6 years ago

Try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=e3b2293cd425b5b2fdf50859abd4ccdcda91970a

	Pulsebot
	Comment 9 • 6 years ago

Pushed by csabou@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/8be0d3702f65
Handle LambdaArrow in MFunctionEnvironment::foldsTo. r=nbp

	Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)
	Comment 10 • 6 years ago
bugherder

https://hg.mozilla.org/mozilla-central/rev/8be0d3702f65