Closed
Bug 1380327
Opened 7 years ago
Closed 7 years ago
gecko_properties.rs is not generated deterministically
Categories
(Core :: CSS Parsing and Computation, defect, P1)
Core
CSS Parsing and Computation
Tracking
()
RESOLVED
FIXED
People
(Reporter: glandium, Assigned: emilio)
References
Details
Attachments
(1 file)
Running two clobber builds in a row, with no changes, using sccache, some crates are getting cache misses.
One of the identified causes of this is the following diff between the objdirs:
--- a/toolkit/library/x86_64-unknown-linux-gnu/release/build/style-844a1337c4fb98af/out/gecko_properties.rs
+++ b/toolkit/library/x86_64-unknown-linux-gnu/release/build/style-844a1337c4fb98af/out/gecko_properties.rs
@@ -4339,7 +4339,7 @@ impl GeckoBorder {
BorderImageSlice {
offsets:
NumberOrPercentageRect::from_gecko_rect(&self.gecko.mBorderImageSlice)
- .expect("mBorderImageSlice[<built-in method index of str object at 0x7fd8924bc8a0>] could not convert to NumberOrP
ercentageRect"),
+ .expect("mBorderImageSlice[<built-in method index of str object at 0x7f7bfa9cf8a0>] could not convert to NumberOrP
ercentageRect"),
fill: self.gecko.mBorderImageFill as u32 == NS_STYLE_BORDER_IMAGE_SLICE_FILL
}
}
@@ -7829,7 +7829,7 @@ impl GeckoOutline {
// NB: This is needed to correctly handling the initial value of
// outline-width when outline-style changes, see the
- // update_border_<_home_glandium_gecko_push_servo_components_style_properties_gecko_mako_rs.Side object at 0x7fd88ef54650> com
ment for more details.
+ // update_border_<_home_glandium_gecko_push_servo_components_style_properties_gecko_mako_rs.Side object at 0x7f7bf7431690> com
ment for more details.
self.gecko.mActualOutlineWidth = self.gecko.mOutlineWidth;
}
Comment hidden (mozreview-request) |
Comment 2•7 years ago
|
||
mozreview-review |
Comment on attachment 8885865 [details]
Bug 1380327: Fix causes of non-determinism in gecko_properties.rs.
https://reviewboard.mozilla.org/r/156632/#review161754
Attachment #8885865 -
Flags: review?(manishearth) → review+
Assignee | ||
Comment 3•7 years ago
|
||
Comment 4•7 years ago
|
||
Probably we cam add a check to build script to fail if there is any "<.+ object at 0x[0-9a-f]+>" inside the generated file, so that we don't regress this.
Reporter | ||
Comment 5•7 years ago
|
||
I can confirm this change allows, on my machine, to have 100% cache hit rate with sccache.
Comment 6•7 years ago
|
||
(In reply to Xidorn Quan [:xidorn] UTC+10 from comment #4)
> Probably we cam add a check to build script to fail if there is any "<.+
> object at 0x[0-9a-f]+>" inside the generated file, so that we don't regress
> this.
Submitted Servo PR servo/servo#17702 to avoid regressing this.
Reporter | ||
Comment 7•7 years ago
|
||
FWIW
https://treeherder.mozilla.org/perf.html#/graphs?series=%5Btry,7c6ec9c4d898dda7736ddd7f0a8ddb9bef63b3cd,1,2%5D&highlightedRevisions=96ef8343d6de
There are 3 retries for that push:
- The first one had a cache miss on style for obvious reasons, since the code was new
- The second one had 100% cache hit, and is the one that is lower than any other cross build on try
- The third one had a cache miss on webrender, which is another problem.
Also, this landed on autoland: https://hg.mozilla.org/integration/autoland/rev/8b8c58abdd34
Reporter | ||
Comment 8•7 years ago
|
||
Comment 9•7 years ago
|
||
Thanks for identifying this problem, glandium, and for fixing the generated code, Emilio!
Looking at build times over the last 14 days, this fix restores incremental automation builds to their pre-Stylo build times:
https://treeherder.mozilla.org/perf.html#/graphs?series=%5Bautoland,4044b74c437dfc672f4615a746ea01f6e4c0312d,1,2%5D&series=%5Bautoland,be465112346255e89dd26003061b01f27cc7fd39,1,2%5D&series=%5Bautoland,7c6ec9c4d898dda7736ddd7f0a8ddb9bef63b3cd,1,2%5D&series=%5Bautoland,d416d6e2ae32b6d43bb1355394d31bfe4fbeee27,1,2%5D&series=%5Bautoland,18851c0bb8875e85890ba4b1aed2439f8c682743,1,2%5D&series=%5Bautoland,be7331d4f74b6970f67fe9da80f4d4d90ef60b73,1,2%5D
Assignee: nobody → emilio+bugs
Priority: -- → P1
Comment 10•7 years ago
|
||
Emilio fixed this in comment 3.
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•