660398 - Speed up test_value_cloning.html

Status: RESOLVED FIXED

(Core :: CSS Parsing and Computation, defect)

Description

[Boris Zbarsky [:bzbarsky]]

Patch coming up that makes this test take 7s in a debug build on my machine instead of 170s.

Comment 1

[Boris Zbarsky [:bzbarsky]]

Attachment: Speed up test_value_cloning. fundamental change is that instead of using 3000+ separate documents to test this we use one big document with distinct IDs for all the tests.

Comment 2

[David Baron :dbaron: (⌚️UTC-5, no longer working on Mozilla)]

Comment on attachment 535795 [details] [diff] [review]
Speed up test_value_cloning.   fundamental change is that instead of using 3000+ separate documents to test this we use one big document with distinct IDs for all the tests.

There are fewer cases to test than there used to be... but could you
test that if you break cloning of some value types (say, by removing the
eCSSUnit_Gradient in nsCSSValue::nsCSSValue) that the relevant tests in
test_value_cloning still fail?

test_value_storage:

  Change serialize_comparatar to serialize_comparator or maybe just to
  serialize_func.  And perhaps where you use it, don't reindent the
  later lines (that's the existing style I've used in such cases, since
  it's temporary).

  And where you change the assignment to |func|, could you write (a ||
  b) ? c : d so I don't have to think about operator precedence?

test_value_cloning:

  Remove the SimpleTest.requestLongerTimeout(4).

r=dbaron with that

Thanks for doing this.  I should have done something like this the first time around.

Comment 3

[Boris Zbarsky [:bzbarsky]]

> say, by removing the eCSSUnit_Gradient in nsCSSValue::nsCSSValue

That codepath is not hit by this test....  Looking into that.

Comment 4

[Boris Zbarsky [:bzbarsky]]

Ah, it's not hit because we only do gradients in background-image and that's a list, so cloning just refcounts the list.

I hacked the "cloning a float value" case to add 0.001 and lots of tests fail as a result, as expected.

Comment 5

[Boris Zbarsky [:bzbarsky]]

> Change serialize_comparatar to serialize_comparator or maybe just to
> serialize_func.

Oops, a typo and then autocomplete... Changed to serialize_func and undid the indent.

> could you write (a || b) ? c : d

Yes.

>  Remove the SimpleTest.requestLongerTimeout(4).

Done.

Pushed http://hg.mozilla.org/projects/cedar/rev/d0423bbf0391

Comment 6

[Mounir Lamouri (:mounir)]

Pushed:
http://hg.mozilla.org/mozilla-central/rev/d0423bbf0391

Comment 7

[Martijn Wargers (dead)]

This test is causing the mochitest run to fail if I run all the layout/style/tests tests, because Fennec is closing during this test. I think it's happening, because this test is using a lot of memory and my phone only has 512MB ram.

Perhaps this test should be broken up in smaller chunks? Or perhaps I should buy more ram?

Comment 8

[Boris Zbarsky [:bzbarsky]]

This test used to be broken up into smaller chunks... that caused it to take forever to run.

That said, 512 megs is a lot more than I recall this test taking when I tested it.  If it's really hitting that much RAM, maybe we should try splitting things up into 2-3 chunks somehow....  Can you file a bug on that, please?

Comment 9

[David Baron :dbaron: (⌚️UTC-5, no longer working on Mozilla)]

That's probably bug 704010, now.