Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Update expected stylo sizes for rust-lang/rust#45225. #19285

Merged
merged 1 commit into from
Nov 21, 2017
Merged

Update expected stylo sizes for rust-lang/rust#45225. #19285

merged 1 commit into from
Nov 21, 2017

Conversation

eddyb
Copy link
Contributor

@eddyb eddyb commented Nov 19, 2017 •
edited by SimonSapin
Loading

See rust-lang/rust#45225 (comment).

  • ./mach build -d does not report any errors
  • ./mach test-tidy does not report any errors
  • These changes fix #__ (github issue number if applicable).
  • There are tests for these changes OR
  • These changes do not require tests because _____

This change is Reviewable

@highfive
Copy link

Heads up! This PR modifies the following files:

  • @bholley: components/style/properties/longhand/pointing.mako.rs, components/style/properties/longhand/inherited_svg.mako.rs
  • @canaltinova: components/style/properties/longhand/pointing.mako.rs, components/style/properties/longhand/inherited_svg.mako.rs
  • @emilio: components/style/properties/longhand/pointing.mako.rs, components/style/properties/longhand/inherited_svg.mako.rs

@highfive highfive added the S-awaiting-review There is new code that needs to be reviewed. label Nov 19, 2017
eddyb added a commit to eddyb/rust that referenced this pull request Nov 19, 2017
@eddyb
cargotest: temporarily use eddyb/servo to include servo/servo#19285.
b0812de
@SimonSapin
Copy link
Member

Thanks @eddyb. This likely won’t pass CI since rust-lang/rust#45225 is not yet in a rustc version we use. I’ll take over to make it conditional or something once rust-lang/rust#45225 lands. (Servo and Firefox use different versions.)

In the meantime, as I said in rust-lang/rust#45225, feel free to disable Stylo tests in Rust CI.

@nox
Copy link
Contributor

nox commented Nov 19, 2017 

failures:
---- size_of::test_size_of_parsed_declaration stdout ----
	thread 'size_of::test_size_of_parsed_declaration' panicked at 'Your changes have increased the stack size of style::properties::SourcePropertyDeclaration from 704 to 872. Please consider choosing a design which avoids this increase. If you feel that the increase is necessary, update the size in /home/travis/build/servo/servo/tests/unit/stylo/size_of.rs.', /home/travis/build/servo/servo/tests/unit/stylo/size_of.rs:53:0
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.
stack backtrace:
   0: std::sys::imp::backtrace::tracing::imp::unwind_backtrace
             at /checkout/src/libstd/sys/unix/backtrace/tracing/gcc_s.rs:49
   1: std::sys_common::backtrace::_print
             at /checkout/src/libstd/sys_common/backtrace.rs:71
   2: std::panicking::default_hook::{{closure}}
             at /checkout/src/libstd/sys_common/backtrace.rs:60
             at /checkout/src/libstd/panicking.rs:380
   3: std::panicking::default_hook
             at /checkout/src/libstd/panicking.rs:390
   4: std::panicking::rust_panic_with_hook
             at /checkout/src/libstd/panicking.rs:611
   5: std::panicking::begin_panic_new
             at /checkout/src/libstd/panicking.rs:553
   6: std::panicking::begin_panic_fmt
             at /checkout/src/libstd/panicking.rs:521
   7: stylo_tests::size_of::test_size_of_parsed_declaration
             at /home/travis/build/servo/servo/ports/geckolib/<size_of_test macros>:9
   8: <F as test::FnBox<T>>::call_box
             at /checkout/src/libtest/lib.rs:1477
             at /checkout/src/libcore/ops/function.rs:143
             at /checkout/src/libtest/lib.rs:138
   9: __rust_maybe_catch_panic
             at /checkout/src/libpanic_unwind/lib.rs:98
---- size_of::test_size_of_property_declaration stdout ----
	thread 'size_of::test_size_of_property_declaration' panicked at 'Your changes have increased the stack size of style::properties::PropertyDeclaration from 32 to 40. Please consider choosing a design which avoids this increase. If you feel that the increase is necessary, update the size in /home/travis/build/servo/servo/tests/unit/stylo/size_of.rs.', /home/travis/build/servo/servo/tests/unit/stylo/size_of.rs:43:0
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.
stack backtrace:
   0: std::sys::imp::backtrace::tracing::imp::unwind_backtrace
             at /checkout/src/libstd/sys/unix/backtrace/tracing/gcc_s.rs:49
   1: std::sys_common::backtrace::_print
             at /checkout/src/libstd/sys_common/backtrace.rs:71
   2: std::panicking::default_hook::{{closure}}
             at /checkout/src/libstd/sys_common/backtrace.rs:60
             at /checkout/src/libstd/panicking.rs:380
   3: std::panicking::default_hook
             at /checkout/src/libstd/panicking.rs:390
   4: std::panicking::rust_panic_with_hook
             at /checkout/src/libstd/panicking.rs:611
   5: std::panicking::begin_panic_new
             at /checkout/src/libstd/panicking.rs:553
   6: std::panicking::begin_panic_fmt
             at /checkout/src/libstd/panicking.rs:521
   7: stylo_tests::size_of::test_size_of_property_declaration
             at /home/travis/build/servo/servo/ports/geckolib/<size_of_test macros>:9
   8: <F as test::FnBox<T>>::call_box
             at /checkout/src/libtest/lib.rs:1477
             at /checkout/src/libcore/ops/function.rs:143
             at /checkout/src/libtest/lib.rs:138
   9: __rust_maybe_catch_panic
             at /checkout/src/libpanic_unwind/lib.rs:98
---- size_of::test_size_of_computed_image_layer stdout ----
	thread 'size_of::test_size_of_computed_image_layer' panicked at 'Your changes have increased the stack size of computed::image::ImageLayer from 40 to 48. Please consider choosing a design which avoids this increase. If you feel that the increase is necessary, update the size in /home/travis/build/servo/servo/tests/unit/stylo/size_of.rs.', /home/travis/build/servo/servo/tests/unit/stylo/size_of.rs:60:0
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.
stack backtrace:
   0: std::sys::imp::backtrace::tracing::imp::unwind_backtrace
             at /checkout/src/libstd/sys/unix/backtrace/tracing/gcc_s.rs:49
   1: std::sys_common::backtrace::_print
             at /checkout/src/libstd/sys_common/backtrace.rs:71
   2: std::panicking::default_hook::{{closure}}
             at /checkout/src/libstd/sys_common/backtrace.rs:60
             at /checkout/src/libstd/panicking.rs:380
   3: std::panicking::default_hook
             at /checkout/src/libstd/panicking.rs:390
   4: std::panicking::rust_panic_with_hook
             at /checkout/src/libstd/panicking.rs:611
   5: std::panicking::begin_panic_new
             at /checkout/src/libstd/panicking.rs:553
   6: std::panicking::begin_panic_fmt
             at /checkout/src/libstd/panicking.rs:521
   7: stylo_tests::size_of::test_size_of_computed_image_layer
             at /home/travis/build/servo/servo/ports/geckolib/<size_of_test macros>:9
   8: <F as test::FnBox<T>>::call_box
             at /checkout/src/libtest/lib.rs:1477
             at /checkout/src/libcore/ops/function.rs:143
             at /checkout/src/libtest/lib.rs:138
   9: __rust_maybe_catch_panic
             at /checkout/src/libpanic_unwind/lib.rs:98
---- size_of::test_size_of_specified_image_layer stdout ----
	thread 'size_of::test_size_of_specified_image_layer' panicked at 'Your changes have increased the stack size of specified::image::ImageLayer from 40 to 48. Please consider choosing a design which avoids this increase. If you feel that the increase is necessary, update the size in /home/travis/build/servo/servo/tests/unit/stylo/size_of.rs.', /home/travis/build/servo/servo/tests/unit/stylo/size_of.rs:61:0
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.
stack backtrace:
   0: std::sys::imp::backtrace::tracing::imp::unwind_backtrace
             at /checkout/src/libstd/sys/unix/backtrace/tracing/gcc_s.rs:49
   1: std::sys_common::backtrace::_print
             at /checkout/src/libstd/sys_common/backtrace.rs:71
   2: std::panicking::default_hook::{{closure}}
             at /checkout/src/libstd/sys_common/backtrace.rs:60
             at /checkout/src/libstd/panicking.rs:380
   3: std::panicking::default_hook
             at /checkout/src/libstd/panicking.rs:390
   4: std::panicking::rust_panic_with_hook
             at /checkout/src/libstd/panicking.rs:611
   5: std::panicking::begin_panic_new
             at /checkout/src/libstd/panicking.rs:553
   6: std::panicking::begin_panic_fmt
             at /checkout/src/libstd/panicking.rs:521
   7: stylo_tests::size_of::test_size_of_specified_image_layer
             at /home/travis/build/servo/servo/ports/geckolib/<size_of_test macros>:9
   8: <F as test::FnBox<T>>::call_box
             at /checkout/src/libtest/lib.rs:1477
             at /checkout/src/libcore/ops/function.rs:143
             at /checkout/src/libtest/lib.rs:138
   9: __rust_maybe_catch_panic
             at /checkout/src/libpanic_unwind/lib.rs:98
---- specified_values::size_of_specified_values stdout ----
	thread 'specified_values::size_of_specified_values' panicked at 'Your changes have increased the size of caret_color SpecifiedValue to 32. The threshold is currently 24. SpecifiedValues affect size of PropertyDeclaration enum and increasing the size may negative affect style system performance. Please consider using `boxed="True"` in this longhand.
Your changes have increased the size of stroke_width SpecifiedValue to 32. The threshold is currently 24. SpecifiedValues affect size of PropertyDeclaration enum and increasing the size may negative affect style system performance. Please consider using `boxed="True"` in this longhand.
Your changes have increased the size of stroke_dashoffset SpecifiedValue to 32. The threshold is currently 24. SpecifiedValues affect size of PropertyDeclaration enum and increasing the size may negative affect style system performance. Please consider using `boxed="True"` in this longhand.', /home/travis/build/servo/servo/tests/unit/stylo/specified_values.rs:48:8
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.
stack backtrace:
   0: std::sys::imp::backtrace::tracing::imp::unwind_backtrace
             at /checkout/src/libstd/sys/unix/backtrace/tracing/gcc_s.rs:49
   1: std::sys_common::backtrace::_print
             at /checkout/src/libstd/sys_common/backtrace.rs:71
   2: std::panicking::default_hook::{{closure}}
             at /checkout/src/libstd/sys_common/backtrace.rs:60
             at /checkout/src/libstd/panicking.rs:380
   3: std::panicking::default_hook
             at /checkout/src/libstd/panicking.rs:390
   4: std::panicking::rust_panic_with_hook
             at /checkout/src/libstd/panicking.rs:611
   5: std::panicking::begin_panic_new
             at /checkout/src/libstd/panicking.rs:553
   6: std::panicking::begin_panic_fmt
             at /checkout/src/libstd/panicking.rs:521
   7: stylo_tests::specified_values::size_of_specified_values
             at ./specified_values.rs:48
   8: <F as test::FnBox<T>>::call_box
             at /checkout/src/libtest/lib.rs:1477
             at /checkout/src/libcore/ops/function.rs:143
             at /checkout/src/libtest/lib.rs:138
   9: __rust_maybe_catch_panic
             at /checkout/src/libpanic_unwind/lib.rs:98
failures:
    size_of::test_size_of_computed_image_layer
    size_of::test_size_of_parsed_declaration
    size_of::test_size_of_property_declaration
    size_of::test_size_of_specified_image_layer
    specified_values::size_of_specified_values

Ms2ger
Ms2ger reviewed Nov 20, 2017
View reviewed changes
tests/unit/stylo/size_of.rs
@@ -57,5 +57,5 @@ size_of_test!(test_size_of_specified_image, specified::image::Image, 40);

// FIXME(bz): These can shrink if we move the None_ value inside the
// enum instead of paying an extra word for the Either discriminant.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this fixed?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think so. That is, we’ll get the shrinkage benefit without changing the type definitions. I’ll double check tomorrow when Rust Nightly has this change, and remove the FIXME comment.

SimonSapin added a commit that referenced this pull request Nov 21, 2017
@SimonSapin
Fix Stylo tests to pass on both Stable and Nightly Rust.
6fc345e 
This is on top of #19285.

Rust Nightly has new enum memory layout optimizations:
rust-lang/rust#45225
@SimonSapin SimonSapin mentioned this pull request Nov 21, 2017
Merged
SimonSapin added a commit that referenced this pull request Nov 21, 2017
@SimonSapin
Fix Stylo tests to pass on both Stable and Nightly Rust.
aaba33e 
This is on top of #19285.

Rust Nightly has new enum memory layout optimizations:
rust-lang/rust#45225
bors-servo pushed a commit that referenced this pull request Nov 21, 2017
Auto merge of #19316 - servo:stylo-size-of, r=emilio
17e97b9 
Fix Stylo tests to pass on both Stable and Nightly Rust

This is on top of #19285.

Rust Nightly has new enum memory layout optimizations: rust-lang/rust#45225

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/19316)
<!-- Reviewable:end -->
@bors-servo bors-servo merged commit 6031de9 into servo:master Nov 21, 2017
@eddyb eddyb deleted the squash branch November 21, 2017 13:33
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this pull request Nov 21, 2017
@SimonSapin
servo: Merge #19316 - Fix Stylo tests to pass on both Stable and Nigh…
6c889a3 
…tly Rust (from servo:stylo-size-of); r=emilio

This is on top of servo/servo#19285.

Rust Nightly has new enum memory layout optimizations: rust-lang/rust#45225

Source-Repo: https://github.com/servo/servo
Source-Revision: 17e97b9320fdb7cdb33bbc5f4d0fde0653bbf2e4

--HG--
extra : subtree_source : https%3A//hg.mozilla.org/projects/converted-servo-linear
extra : subtree_revision : 3255b68a8069a815effae1b6b6ee4e97dedaddd3
xeonchen pushed a commit to xeonchen/gecko-cinnabar that referenced this pull request Nov 22, 2017
@SimonSapin
servo: Merge #19316 - Fix Stylo tests to pass on both Stable and Nigh…
19dbc7b 
…tly Rust (from servo:stylo-size-of); r=emilio

This is on top of servo/servo#19285.

Rust Nightly has new enum memory layout optimizations: rust-lang/rust#45225

Source-Repo: https://github.com/servo/servo
Source-Revision: 17e97b9320fdb7cdb33bbc5f4d0fde0653bbf2e4
JerryShih pushed a commit to JerryShih/gecko-dev that referenced this pull request Nov 28, 2017
@SimonSapin
servo: Merge #19316 - Fix Stylo tests to pass on both Stable and Nigh…
15f2124 
…tly Rust (from servo:stylo-size-of); r=emilio

This is on top of servo/servo#19285.

Rust Nightly has new enum memory layout optimizations: rust-lang/rust#45225

Source-Repo: https://github.com/servo/servo
Source-Revision: 17e97b9320fdb7cdb33bbc5f4d0fde0653bbf2e4
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified-and-comments-removed that referenced this pull request Oct 2, 2019
@marco-c
servo: Merge #19316 - Fix Stylo tests to pass on both Stable and Nigh…
4dec72d 
…tly Rust (from servo:stylo-size-of); r=emilio

This is on top of servo/servo#19285.

Rust Nightly has new enum memory layout optimizations: rust-lang/rust#45225

Source-Repo: https://github.com/servo/servo
Source-Revision: 17e97b9320fdb7cdb33bbc5f4d0fde0653bbf2e4

UltraBlame original commit: 6de571030d5d998dcadbd3dac602fa006395165c
gecko-dev-updater pushed a commit to marco-c/gecko-dev-comments-removed that referenced this pull request Oct 2, 2019
@marco-c
servo: Merge #19316 - Fix Stylo tests to pass on both Stable and Nigh…
11f54f6 
…tly Rust (from servo:stylo-size-of); r=emilio

This is on top of servo/servo#19285.

Rust Nightly has new enum memory layout optimizations: rust-lang/rust#45225

Source-Repo: https://github.com/servo/servo
Source-Revision: 17e97b9320fdb7cdb33bbc5f4d0fde0653bbf2e4

UltraBlame original commit: 6de571030d5d998dcadbd3dac602fa006395165c
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified that referenced this pull request Oct 2, 2019
@marco-c
servo: Merge #19316 - Fix Stylo tests to pass on both Stable and Nigh…
7fe1e8b 
…tly Rust (from servo:stylo-size-of); r=emilio

This is on top of servo/servo#19285.

Rust Nightly has new enum memory layout optimizations: rust-lang/rust#45225

Source-Repo: https://github.com/servo/servo
Source-Revision: 17e97b9320fdb7cdb33bbc5f4d0fde0653bbf2e4

UltraBlame original commit: 6de571030d5d998dcadbd3dac602fa006395165c
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@SimonSapin SimonSapin SimonSapin left review comments

@Ms2ger Ms2ger Ms2ger left review comments

Assignees

@cbrewster cbrewster

Labels
S-awaiting-review There is new code that needs to be reviewed.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@eddyb @highfive @SimonSapin @nox @Ms2ger @bors-servo @cbrewster