607201 - Slower than v8 in the second testcase in bug 606650

Status: RESOLVED WORKSFORME

(Core :: JavaScript Engine, defect)

Description

[Boris Zbarsky [:bzbarsky]]

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.

Comment 1

[Nicholas Nethercote [inactive]]

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.

Comment 2

[Boris Zbarsky [:bzbarsky]]

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).

Comment 3

[Nicholas Nethercote [inactive]]

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!

Comment 4

[Boris Zbarsky [:bzbarsky]]

> 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.

Comment 5

[Ryan VanderMeulen [:RyanVM]]

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.

Comment 6

[Boris Zbarsky [:bzbarsky]]

Probably needs retesting...

Comment 7

[Guilherme Lima]

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

Thanks BZ!

Comment 8

[Tom S [:evilpie]]

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

Comment 9

[Guilherme Lima]

What's the difference? You have a faster machine... your Nightly was 25% faster than mine, and your Chromium 17% faster than my Chrome.

Comment 10

[Boris Zbarsky [:bzbarsky]]

Guilherme, thanks!