Closed Bug 70054 Opened 23 years ago Closed 15 years ago

Nested function invocation too slow, Part Deux

Categories

(Core :: JavaScript Engine, enhancement)

enhancement
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla1.1alpha

People

(Reporter: brendan, Unassigned)

References

Details

(Keywords: perf)

Attachments

(1 file)

shaver wrote in bug 65308:
>For future consideration:
> - to make JSOP_CLOSURE not be the heavy, we need to <brendan>avoid a 
>JSOP_DEFVAR when there is a var and a closure in a function, *and* emit a
>JSOP_DEFLOCALFUN at the closure site</brendan>
> - we could use a bit in sprop->spare (``they're just _sitting_ there'') to
>flag a collision betwen var and closure, rather than putting the whole
>function out of bounds.

I should say that even with JS_DOUBLE_HASHING (bug 62164), sprop->reserved is
"just _sitting_ there".

I had another crazy idea: add a prefix bytecode (a la JSOP_GETTER and
JSOP_SETTER) called JSOP_UP, taking an immediate byte, of value |n|, telling how
many scopes to go up in the scope chain, that can precede all JOF_QVAR and
JOF_QARG bytecodes.  It causes those codes to use a JSStackFrame * recovered
from the private data of the Call object after skipping |n| (or maybe |n+1| to
use the zero value of the immediate well) objects on fp->scopeChain.

If the call object |n| up has no private data, then its JSStackFrame must have
been popped as control unwound from the call.  In that case, the local vars and
args have been snapshotted (call_enumerate, from js_PutCallObject) and must be
accessed by name.

This doesn't avoid heavyweight outers when non-local names are used, it merely
makes access faster (by allowing stack slots to be accessed via frame pointers).
It also doesn't win if the outers are no longer active when an inner is invoked,
so maybe JSOP_UP+quickop shouldn't be generated at all, if the compiler can
determine that no calls to an inner come from its outer (in particular, that no
inner function reference leaks to an outer -- ignore the compiler-created inner
property in outer!).

/be
Keywords: perf
Marking future.
Target Milestone: --- → Future
Why are you moving my bugs around, Patrick?

I'd like to get this done for 1.0, myself.
Severity: normal → enhancement
Status: NEW → ASSIGNED
Target Milestone: Future → mozilla1.0
Triage to 1.1: I don't think this is a bug win for us, though I'm happy to
entertain analysis that indicates otherwise.
Target Milestone: mozilla1.0 → mozilla1.1
I'll say that this tool

    http://peds.wustl.edu/div/gi/spec/tpn/tpn_calculator.html

is impossible to use on mozilla (not sure if this bug is the cause) because it's
too slow. Our hospital intranet uses NN 4.7x (on solaris) which works fine. NN4
on Mac/PC and MSIE on mac/pc also handle this tool just fine.

To use, enter in an age/wheight/height, and wait for default to fill in all the
way down in the electrolytes section. It's practically instantaneous in other
browsers.

Try 3mo, 4kg, 56cm... don't tab out of the 56cm, but instead scroll down to the
electrolytes section and then click somewhere in the page background down there.
Now wait.... and wait...

-matt
Not sure if Matt's site is caused by this bug or not. There are lots of
DOM events occurring, etc., etc. At any rate, I'll attach a javascript:URL
that you can use to recreate Matt's steps in Comment #4. Just load 
http://peds.wustl.edu/div/gi/spec/tpn/tpn_calculator.html, enter the 
javascript:URL, and you will get an alertbox with a timing result.

Here are my results on WinNT (128M RAM, 500MHz processor)
(My Mozilla build was 20020222xx; all times in milliseconds):

                NN4.7     IE6     Mozilla 
                 16       200       900

Matt, what is your Mozilla build ID, and what timing do you get?

Note the remarkable win by NN4.7 here. My own feeling is, since NN4.7
is so fast on this, and AFAIK JS Engine performance has not regressed
since then, the slow performance of Mozilla must be due to the significant 
changes in the browser that have occurred since then - sound right?
If so, this should be filed as a  separate bug on DOM performance,
not against JS Engine -

I could try to convert this site into a pure JS Engine testcase to be sure,
but that will take a lot of time. If anyone thinks I should do that now,
please let me know.
Matt: why do you think that page's performance is related to nested function
performance, which has, to my knowledge, only improved since Netscape 4.x?  I
don't see any nested functions in there at all.

Phil: I think you're right, and Matt's complaint is about DOM performance.
Re: DOM performance
    I had missunderstood this bug to be something like
    
        > JavaScript execution is too low when function
        > call stack gets too deep
    
    and so thought that functions calling functions which
    call other functions (ad naseum) was what was slow

    Is there an existing bug for the DOM/JS issue?

Re: my observations.

    I tried the JS timer and found the following

                        MSIE 5.1 on OS X  8106
        Mozilla build 2002030403 on OS X  4228
             NN 4.78 on OS X via classic  25

    I was suprised by MSIE's time. My previous
    observations about MSIE being about as fast as
    NN were based on PC versions (which I can't
    test right at this moment).

-matt
Function execution performance shouldn't be affected by current calling depth,
other than by third-order effects like memory pressure created by extreme stack
allocation.  But do you see unusually deep call chains on that page?

Search for "DHTML performance" to find piles of reports of bugs that are almost
certainly just like yours.
I have filed bug 128901 against the DOM for the performance problem at
the tpn_calculator.html site, and have cc'ed Matt on it. Note, however,
the DOM folks may dupe this against the main tracking bug for DHTML
performance, bug 21762. If they do, Matt may wish to cc himself on
that one, too -
thanks Phil :)

:
:

re: "But do you see unusually deep call chains on that page?"

I don't think they're so deep as to cause memory issues, but I would classify
them as "unusually deep". On the other hand, my ideal of unusually deep in a
JavaScript tool may be very skewed as I don't consider myself a JavaScript
developer. The stacks are deeper than in another JavaScript thing I've done. If
you're sufficiently curious, I can try to create a version that counts max
depth. Just let me know if that would be helpfull...

-matt
after the fix for bug 129115, the calculation is near instant
Testing on XP, trunk 2002051104, P4/1.8
To the orphanage.
Assignee: shaver → nobody
Status: ASSIGNED → NEW
QA Contact: pschwartau → general
Blocks: 117611
We did this with upvar!
Status: NEW → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: