169278 - tracking "patch to try that localizes rt->interruptHandler" - attachment 92768's lifespan

Status: VERIFIED FIXED

(Core :: JavaScript Engine, defect)

Description

[timeless]

split off from Bug 121414 JS needs partial evaluation

------- Additional Comment #35 From Brendan Eich  2002-07-25 11:47 -------
Created an attachment (id=92768)
patch to try that localizes rt->interruptHandler

This patch loads rt->interruptHandler into a local before the interpreter loop,
and reloads it only after returns from js_Invoke (which might call a native
function).  It may not help, if optimizing compilers and target architectures
don't lead to the interruptHandler local being kept in a register.

------- Additional Comment #36 From Yuedong Du 2002-07-26 00:55 -------
The patch is valuable at least on solaris:
with the testcase test1(sun blade 100, 500Mhz, solaris 8, average of 3 times):

with the patch      : 2251 ms
without the patch   : 2595 ms

------- Additional Comment #37 From Yuedong Du 2002-07-26 05:34 -------
Created an attachment (id=92885)
quantify solaris remove js debugger support

quantified as /be requested.

------- Additional Comment #39 From Yuedong Du 2002-07-29 04:00 -------
And the patch of Brendan is obviously beneficial. So can we let patch go through
the review process? 

We have proved that bug has nothing to do with bitwise operator. Maybe switch is
too heavy but no obvious way for leveage the overhead. So the patch is probably
the only fruit of this bug.

------- Additional Comment #40 From Brendan Eich 2002-07-30 15:44 -------
Taking.

------- Additional Comment #44 From John Bandhauer 2002-07-31 23:09 -------
(From update of attachment 92768 [details] [diff] [review])
sr=jband. Sure, why not? I'm curious if it really measures as a win.

------- Additional Comment #45 From Robert Ginda 2002-07-31 23:20 -------
(From update of attachment 92768 [details] [diff] [review])
r=rginda
Doesn't seem to cause any debugger regressions, afaict.

------- Additional Comment #46 From Brendan Eich 2002-08-01 11:08 -------
jband: see comment #36 for YueDong Du's measurement with the patch vs. without.

I'll check the patch in when the 1.2alpha trunk opens, but keep the bug open for
bigger improvements.

------- Additional Comment #47 From Phil Schwartau 2002-08-13 15:24 -------
The latest patch passes the JS testsuite on WinNT in both the
debug and optimized shells. No test regressions occurred -

------- Additional Comment #48 From Brendan Eich 2002-08-13 17:28 -------
I checked attachment 92768 [details] [diff] [review] into the 1.2alpha trunk.

------- Additional Comment #49 From Henry Jia 2002-08-28 20:33 -------
In OEM branch.

Comment 1

[timeless]

.

Comment 2

[Brendan Eich [:brendan]]

timeless, you should take this bug if you're the one driving this fix into the
1.0 branch.  I was planning to land the whole js1.5 engine sources from the
trunk onto the 1.0 branch, later (next month?).

If you just filed this to split it out for posterity, I'm not sure it was worth
it.  I guess pschwartau has to reVERIFY it.

/be

Comment 3

[Phil Schwartau]

Marking Verified -