Last Comment Bug 660398 - Speed up test_value_cloning.html
: Speed up test_value_cloning.html
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: CSS Parsing and Computation (show other bugs)
: Trunk
: All All
: -- normal (vote)
: mozilla7
Assigned To: Boris Zbarsky [:bz]
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2011-05-27 18:17 PDT by Boris Zbarsky [:bz]
Modified: 2011-11-20 17:42 PST (History)
5 users (show)
bzbarsky: in‑testsuite-
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
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. (12.23 KB, patch)
2011-05-27 18:28 PDT, Boris Zbarsky [:bz]
dbaron: review+
Details | Diff | Review

Description Boris Zbarsky [:bz] 2011-05-27 18:17:41 PDT
Patch coming up that makes this test take 7s in a debug build on my machine instead of 170s.
Comment 1 Boris Zbarsky [:bz] 2011-05-27 18:28:21 PDT
Created 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.
Comment 2 David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) 2011-05-29 03:08:33 PDT
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 [:bz] 2011-06-01 09:24:04 PDT
> say, by removing the eCSSUnit_Gradient in nsCSSValue::nsCSSValue

That codepath is not hit by this test....  Looking into that.
Comment 4 Boris Zbarsky [:bz] 2011-06-01 10:09:15 PDT
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 [:bz] 2011-06-01 13:02:17 PDT
> 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) 2011-06-02 04:16:12 PDT
Pushed:
http://hg.mozilla.org/mozilla-central/rev/d0423bbf0391
Comment 7 Martijn Wargers [:mwargers] (gone per 2016-05-31 :-( ) 2011-08-11 08:20:39 PDT
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 [:bz] 2011-08-11 08:47:24 PDT
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-7 (review requests must explain patch) 2011-11-20 17:42:59 PST
That's probably bug 704010, now.

Note You need to log in before you can comment on or make changes to this bug.