Closed Bug 1154339 Opened 9 years ago Closed 8 years ago

JS microbenchmark with JQuery proxy is much slower when inner callback function uses tabs instead of spaces for indentation

Categories

(Core :: JavaScript Engine, defect)

37 Branch
defect
Not set
normal

Tracking

()

RESOLVED FIXED
Tracking Status
platform-rel --- +

People

(Reporter: aladjev.andrew, Assigned: jandem)

References

Details

(Keywords: reproducible, testcase, Whiteboard: [platform-rel-jQuery])

Please open http://jsfiddle.net/1qhsL84w/1/ than open firebug, press "run". The result will be "5-15".
Please open http://jsfiddle.net/1qhsL84w/2/ than open firebug, press "run". The result will be "0".

There are 2 scripts:
http://googledrive.com/host/0B11zLXPSukRzdmYyRndDQlRhT00 is original jquery-1.10.2
http://googledrive.com/host/0B11zLXPSukRzZTNyeUVDNVVldGc is a bit modified jquery-1.10.2

--- 0B11zLXPSukRzdmYyRndDQlRhT00
+++ 0B11zLXPSukRzZTNyeUVDNVVldGc
@@ -824,7 +824,7 @@
 		// Simulated bind
 		args = core_slice.call( arguments, 2 );
 		proxy = function() {
-			return fn.apply( context || this, args.concat( core_slice.call( arguments ) ) );
+            return fn.apply( context || this, args.concat( core_slice.call( arguments ) ) );
 		};

Original jquery-1.10.2 have "\t\t\t" (3 tabs), modified have "            " (12 spaces).

This looks like a serious bug in your js engine. How white space in script can affect the performance of this script?
Please checkout http://jsfiddle.net/1qhsL84w/3/ and http://jsfiddle.net/1qhsL84w/4/ too. The difference is significant more.
You can disable "script" tab in firebug and bug will disappear. It looks like "script" tab uses some buggy method from javascript SDK for addons.
Can you reproduce this still, on an up-to-date Firefox, with the builtin developer tools?
Component: General → Untriaged
Flags: needinfo?(aladjev.andrew)
Yes. I've installed new firefox without any extensions in new clean windows 7 virtual machine.

Please click here: http://jsfiddle.net/1qhsL84w/7/ and http://jsfiddle.net/1qhsL84w/6/
I have 234 and 50 miliseconds. The diff between these scripts is 3 tabs and 12 spaces.
Flags: needinfo?(aladjev.andrew)
I'm flummoxed. Jan, can you or someone else on the JS team take a look? You don't need firebug - I can reproduce on plain 45 release as well as Nightly (both on win8)
Status: UNCONFIRMED → NEW
Component: Untriaged → JavaScript Engine
Ever confirmed: true
Flags: needinfo?(jdemooij)
Product: Firefox → Core
Summary: JQuery proxy is extremely slow with firebug opened → JS microbenchmark with JQuery proxy is much slower when inner callback function uses tabs instead of spaces for indentation
I will forward this issue to jorendorff, I think he might know better the frontend of SpiderMonkey.
Flags: needinfo?(jdemooij) → needinfo?(jorendorff)
I am able to reproduce this issue even when the jit are disabled.
You have got to be kidding me.
> This looks like a serious bug in your js engine. How white space in script can affect the performance
> of this script?

This is extremely surprising. I can't imagine what would cause this. Looking into it.
Assignee: nobody → jorendorff
Flags: needinfo?(jorendorff)
This is likely caused by the last check in ObjectGroup::useSingletonForClone :/
I had a bug that website work extremely slow only in firefox. I've figured out that jquery.proxy was a bottleneck. I've upgraded jquery to latest version and the bug worked perfect. I've pretty printed jquery.js with spaces and got ready to ... but the bug disappeared. Than I've started to replace spaces with tabs line by line and found that the problem was in tabs around line:

> return fn.apply( context || this, args.concat( core_slice.call( arguments ) ) );

This looks absurdly, but it works.

PS Firebug's script tab somehow amplifies this bug.
Andrew, we have an optimization for fn.apply() that only kicks in if the function that is calling fn.apply() is very short: 100 characters or less.

Does this agree with what you've observed?

(We do this because the cost of the optimization itself is proportional to the size of that function - the "optimized" version can even be slower in some cases. Admittedly measuring how long a function is by counting characters is goofy.)
The fact that the tabs version is slower here makes me think the "optimization" is backfiring. It would make sense, as the microbenchmark creates many instances of this function.
Jan has a patch that might help here. Assigning the bug to him.
Assignee: jorendorff → jdemooij
(In reply to Jason Orendorff [:jorendorff] from comment #14)
> Jan has a patch that might help here. Assigning the bug to him.

Bug 1227035. I wrote a patch for it yesterday, weird coincidence.
Depends on: 1227035
Whiteboard: [platform-rel-jQuery]
platform-rel: --- → ?
platform-rel: ? → +
I this still an issue?
Flags: needinfo?(jdemooij)
I just tested with the jsFiddles from comment #4 and got consistent results in both independent of the developer tools being open.

Firefox 47.0.1 under Linux
(In reply to Boris Zbarsky [:bz] from comment #16)
> I this still an issue?

The patch for bug 1227035 should have fixed this one. The bad TI heuristic is still there; unfortunately fixing that will require more thought/work.
Flags: needinfo?(jdemooij)
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.