Slower than v8 in the second testcase in bug 606650

RESOLVED WORKSFORME

Status

()

Core
JavaScript Engine
RESOLVED WORKSFORME
7 years ago
4 years ago

People

(Reporter: bz, Unassigned)

Tracking

(Blocks: 1 bug, {regression})

Trunk
x86
Mac OS X
regression
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(URL)

This seems like reasonable code for people to write, and TM does a lot better than JM on it.

I believe the basic problem is all the branches, but in this case we can statically prove that only one path is taken...  Nicholas, can we do this with interval analysis somehow?  The goal would be to teach the profiler that the branch conditions are effectively constant.
Blocks: 580468
It's unclear to me which loop(s) in that test (the "ripple" one, right?) is the important one.  Can you elaborate, or even provide a cut-down test case?

Nanojit has a basic interval analysis, but I'm not sure if that is any help with the profiling.
Nicholas, as far as I can tell the relevant loops are the ones in processBuffers.  The issue is that the inner one makes a bunch of sequential calls to get(), which looks like this:

        if (x > 0 && x < this.width && y > 0 && y < this.height) {
            return this.data[x][y];
        } else {
            return 0;
        }

Since they come one after another, we think we have a branching factor of 2^(# of calls) and profiling blacklists the loop.

The key part here is that the loops are:

		for (var x = 2; x < this.width - 2; x++) {
			for (var y = 2; y < this.height - 2; y++) {

so the condition in get() always tests true (or would if it used >= 0 instead of > 0.... but I doubt making that change helps right now).
I think this is essentially the same problem as the one that bug 607874 laments -- the tracer doesn't know anything about the loop variables 'x' and 'y'.  If it did, Nanojit's interval analysis would totally eat those tests up.  (That's assuming the tracer doesn't decide "too many branches" before Nanojit even enters the picture.)(In reply to comment #0)

> This seems like reasonable code for people to write

Not sure I agree with that.  I'm all for writing defensive code, but this is overkill!
> I'm all for writing defensive code, but this is overkill!

Well, the case when I see this happening is when the caller and callee are written by different people (e.g. webpage and library respectively).

This case is easier than bug 607874 because it doesn't have any free variables involved.  In any case, this may not be worth worrying about, esp. if we can make JM smarter here.
Interp: 12803
TM: 12768
JM: 750
JM+TI: 609
d8: 499

JM+TI made testcase 2 from bug 606650 faster, but v8 is still about 1.2x faster.
Blocks: 467263
Summary: Figure out whether we can trace the second testcase in bug 606650 → Slower than v8 in the second testcase in bug 606650
Probably needs retesting...

Comment 7

4 years ago
Using the testcase from the URL field:
Firefox 24 - 504ms
Chrome 29 - 496ms
Nightly 26.0a1 (2013-09-15) - 214ms

Thanks BZ!
For me it looks different:
Nightly: 162ms
Chromium 28: 414ms

Comment 9

4 years ago
What's the difference? You have a faster machine... your Nightly was 25% faster than mine, and your Chromium 17% faster than my Chrome.
Guilherme, thanks!
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → WORKSFORME
You need to log in before you can comment on or make changes to this bug.