603418 - Changeset edb0506b0c23 caused a 4ms regression on SunSpider

Status: RESOLVED INVALID

(Core :: JavaScript Engine, defect)

Description

[David Mandelin [:dmandelin]]

This changeset:

changeset:   55189:edb0506b0c23
user:        Luke Wagner <[e-mail]>
date:        Fri Oct 08 22:58:33 2010 -0700
summary:     Remove unnecessary padding from JSStackFrame

came with a 3.7ms regression on AWFY. Testing locally on Windows, I got a 4.0ms regression.

Comment 1

[David Mandelin [:dmandelin]]

I tested this again, with --runs-50, and got a 4 ms *improvement* on my Windows machine. So it might be noise. Needs more testing on other machines.

Comment 2

[Robert Sayre]

aiui, this patch was landed to make room for a member to be added by the compartments work. I think this alignment issue is GCC specific.

Comment 3

[David Anderson [:dvander] - inactive, e-mail if emergency]

That was JSScript, this looks like JSStackFrame

Comment 4

[Andreas Gal :gal]

Yes, we landed the patch that adds an element so we should have the original alignment back.

Comment 5

[David Mandelin [:dmandelin]]

Cool, this shouldn't have any effect then. Btw, I also saw a 2ms regression from this changeset to after-the-merge on AWFY, but it doesn't duplicate locally. So it's probably all noise.

Comment 6

[Paul Wright]

Based on the consistency of AWFY when no "perf" changes are made, I cannot agree that a ~4 ms delta is "in the noise".  If it were noise, one would expect to see the graph return to the "improved" state on subsequent points.  The graph, as it stands now, shows no such return to that improved condition.

Comment 7

[Luke Wagner [:luke]]

Its also worth nothing that the same changeset made V8 30.8ms (1.2%) faster.

Comment 8

[Paul Wright]

Yes, and this validates the hypothesis that there was, in fact, a shift in the data.  Thanks for that additional input, Luke.

Comment 9

[Nicholas Nethercote [inactive]]

Assuming that alignment isn't stuffed up, making structures smaller should always be a good thing, even if it causes a temporary slowdown on some machines.

I suspect the AWFY slowdown will disappear eventually, but it may take a few commits to do so.