1449148 - Move the test data of TestStrings.cpp into a fixture

Status: RESOLVED FIXED

(Core :: XPCOM, enhancement)

Description

[Henri Sivonen (:hsivonen)]

Shouldn't do disk reads from the timed benchmarks.

Comment 1

[Henri Sivonen (:hsivonen)]

Attachment: Bug 1449148 - Move the test data of TestStrings.cpp into a fixture.

Review commit: https://reviewboard.mozilla.org/r/231544/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/231544/

Comment 2

[Henri Sivonen (:hsivonen)]

Intentionally added both UTF-8 and UTF-16 versions of every test string to make subsequent test additions easy.

Comment 3

[Henri Sivonen (:hsivonen)]

Comment on attachment 8962682 [details]
Bug 1449148 - Move the test data of TestStrings.cpp into a fixture.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/231544/diff/1-2/

Comment 4

[Henri Sivonen (:hsivonen)]

The benchmark results change in non-obvious ways. We just need to treat the situation after this as the new baseline.

Comment 5

[Nathan Froyd [:froydnj]]

Comment on attachment 8962682 [details]
Bug 1449148 - Move the test data of TestStrings.cpp into a fixture.

https://reviewboard.mozilla.org/r/231544/#review237162

Thank you!  I agree that any performance changes we get from this, regressions or otherwise, will just have to become the new baseline.

::: xpcom/tests/gtest/TestStrings.cpp:1449
(Diff revision 2)
> -#define TestExample4 " Donec feugiat volutpat massa. Cras ornare lacinia porta. Fusce in feugiat nunc. Praesent non felis varius diam feugiat ultrices ultricies a risus. Donec maximus nisi nisl, non consectetur nulla eleifend in. Nulla in massa interdum, eleifend orci a, vestibulum est. Mauris aliquet, massa et convallis mollis, felis augue vestibulum augue, in lobortis metus eros a quam. Nam              ac diam ornare, vestibulum elit sit amet, consectetur ante. Praesent massa mauris, pulvinar sit amet sapien vel, tempus gravida neque. Praesent id quam sit amet est maximus molestie eget at turpis. Nunc sit amet orci id arcu dapibus fermentum non eu erat.\f\tSuspendisse commodo nunc sem, eu congue eros condimentum vel. Nullam sit amet posuere arcu. Nulla facilisi. Mauris dapibus iaculis massa sed gravida. Nullam vitae urna at tortor feugiat auctor ut sit amet dolor. Proin rutrum at nunc et faucibus. Quisque suscipit id nibh a aliquet. Pellentesque habitant morbi tristique senectus et netus et malesuada fames ac turpis egestas. Aliquam a dapibus erat, id imperdiet mauris. Nulla blandit libero non magna dapibus tristique. Integer hendrerit imperdiet lorem, quis facilisis lacus semper ut. Vestibulum ante ipsum primis in faucibus orci luctus et ultrices posuere cubilia Curae Nullam dignissim elit in congue ultricies. Quisque erat odio, maximus mollis laoreet id, iaculis at turpis. "
> -
> -#define TestExample5 "Donec id risus urna. Nunc consequat lacinia urna id bibendum. Nulla faucibus faucibus enim. Cras ex risus, ultrices id semper vitae, luctus ut nulla. Sed vehicula tellus sed purus imperdiet efficitur. Suspendisse feugiat\n\n\n     imperdiet odio, sed porta lorem feugiat nec. Curabitur laoreet massa venenatis\r\n risus ornare\r\n, vitae feugiat tortor accumsan. Lorem ipsum dolor sit amet, consectetur adipiscing elit. Maecenas id scelerisque mauris, eget facilisis erat. Ut nec pulvinar risus, sed iaculis ante. Mauris tincidunt, risus et pretium elementum, leo nisi consectetur ligula, tincidunt suscipit erat velit eget libero. Sed ac est tempus, consequat dolor mattis, mattis mi. "
> -
>  // Note the five calls in the loop, so divide by 100k
> -MOZ_GTEST_BENCH(Strings, PerfStripWhitespace, [] {
> +MOZ_GTEST_BENCH_F(Strings, PerfStripWhitespace, [this] {

These tests are a little better after using fixture setups, I guess, but they're still a little busted. :(

Comment 6

[Henri Sivonen (:hsivonen)]

Comment on attachment 8962682 [details]
Bug 1449148 - Move the test data of TestStrings.cpp into a fixture.

https://reviewboard.mozilla.org/r/231544/#review237162

Thanks.

> These tests are a little better after using fixture setups, I guess, but they're still a little busted. :(

Maybe we should use BlackBox around the inputs for these.

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/bb66946e1e6e
Move the test data of TestStrings.cpp into a fixture. r=froydnj

Comment 8

[Eliza Balazs [:ebalazs_]]

https://hg.mozilla.org/mozilla-central/rev/bb66946e1e6e

Comment 9

[Ionuț Goldan [:igoldan]]

This bug influenced at least one baseline:

== Change summary for alert #12394 (as of Tue, 27 Mar 2018 16:04:16 GMT) ==

Improvements:

 13%  Strings PerfIsUTF8One windows7-32 opt      1,276.42 -> 1,109.50

For up to date results, see: https://treeherder.mozilla.org/perf.html#/alerts?id=12394