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.

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

Add binary size testing #140

Closed
sffc opened this issue Jun 19, 2020 · 8 comments · Fixed by #871
Closed

Add binary size testing #140

sffc opened this issue Jun 19, 2020 · 8 comments · Fixed by #871
Assignees
Labels
A-performance Area: Performance (CPU, Memory) C-test-infra Component: Integration test infrastructure S-medium Size: Less than a week (larger bug fix or enhancement) T-core Type: Required functionality
Milestone

Comments

@sffc
Copy link
Member

sffc commented Jun 19, 2020

In each component, we should have a set of demo files that illustrate common use patterns. We should then compile those demo files with optimizations for code size (-s), and have a test that reports their sizes and complains if they grow.

I suggest building the demo files to WebAssembly and checking the size of the output .wasm file, because:

  1. WebAssembly is not platform-dependent
  2. WebAssembly has good tooling to inspect what causes size regressions to occur
  3. We have WebAssembly in mind as a possible target in the future
@sffc sffc added T-core Type: Required functionality C-test-infra Component: Integration test infrastructure labels Jun 19, 2020
@zbraniecki
Copy link
Member

I think cargo /examples may be a good vehicle for such little demo apps. Also, it's related to #107 and maybe we could visualize it via #117.

@sffc
Copy link
Member Author

sffc commented Jun 22, 2020

Sounds good. I can't seem to find the official documentation for the examples/ directory, but I found it mentioned in a few places:

https://www.google.com/books/edition/_/kOiZDwAAQBAJ?hl=en&gbpv=1&pg=PA66&dq=cargo+examples+directory

@eyeplum
Copy link

eyeplum commented Jun 23, 2020

Sounds good. I can't seem to find the official documentation for the examples/ directory

This and this might explain examples/ a bit more :)

@sffc sffc added this to the 2020 Q3 milestone Jun 24, 2020
@zbraniecki zbraniecki added the help wanted Issue needs an assignee label Oct 15, 2020
@sffc sffc modified the milestones: 2020 Q3, 2020 Q4 Oct 22, 2020
@sffc sffc assigned nciric and unassigned echeran Oct 22, 2020
@sffc
Copy link
Member Author

sffc commented Nov 19, 2020

With #365 merged, this issue is unblocked. We can test how the size of the *.wasm files changes across PRs.

@sffc sffc assigned gnrunge and unassigned nciric Dec 4, 2020
@sffc sffc removed the help wanted Issue needs an assignee label Dec 4, 2020
@sffc sffc modified the milestones: 2020 Q4, ICU4X 0.2 Dec 4, 2020
@gnrunge
Copy link
Contributor

gnrunge commented Dec 14, 2020

Questions of a newcomer:

  • Do the demo files mentioned above exist? What are the common use pattern?
  • How to build the compiled demo files?
  • Will GHA be a suitable platform for this CI task? GHA is stateless, isn't it?

@sffc
Copy link
Member Author

sffc commented Dec 14, 2020

The demo files are found in the examples/ directories in each component. You can build them to WASM by running cargo make wasm, and from there you can measure their file size.

I imagine the file size should be recorded in a manner similar to how we record performance data (cc @echeran).

@echeran
Copy link
Contributor

echeran commented Dec 14, 2020

Yes, I expect binary size testing would be similar to performance testing. Ideally, the manifestation of the information would be a dashboard that allows us to notice changes over time. The reason is the same for performance testing -- it is easier to help us determine whether a change in metric value is reasonable through comparison to previous values than it would be assessing the absolute number directly.

For dashboards, we have a high-level overview of how the dashboard action works and inline source comments. I think that we could reuse the dashboard, but instead of providing it a performance metric number, we provide it a different number (binary size), and inherit the benefits of the dashboard 'infrastructure' provided by the GH Action.

Specifically, this would be providing our own value to the tool parameter of the benchmark dashboard GH Action to represent binary size testing. Seeing that the tool parameter is required and must be provided from a short list of options that represent supported performance testing tools, we would need either:

  1. The benchmark action repo to genercize its notion of a benchmark tool
  2. Fork the benchmark action repo and modify it to have support for binary size numbers. It might be similar to how people have created working support for benchmarkdotnet via their fork, but perhaps with less logic required. At first glance, it seems like the core of the work is mimic the JSON format consumed directly by the benchmark action (and thereby skip the benchmark output -> JSON conversion) followed by enabling a new corresponding option for the tool parameter.

@sffc sffc modified the milestones: ICU4X 0.2, 2021-Q1-m2 Jan 7, 2021
@sffc sffc modified the milestones: 2021-Q1-m2, 2021-Q2-m1 Mar 12, 2021
@sffc sffc modified the milestones: 2021-Q2-m1, ICU4X 0.3 Apr 19, 2021
@sffc sffc added the S-medium Size: Less than a week (larger bug fix or enhancement) label Apr 19, 2021
@sffc sffc added the A-performance Area: Performance (CPU, Memory) label Apr 30, 2021
@gnrunge
Copy link
Contributor

gnrunge commented Jul 15, 2021

Draft PR: #859

gnrunge pushed a commit to gnrunge/icu4x that referenced this issue Jul 20, 2021
…amples

compiled into wasm binaries. Set up GHA to build wasm binaries, measure
file sizes, push results into benchmark dashboard .

Resolves ticket unicode-org#140.
gnrunge pushed a commit to gnrunge/icu4x that referenced this issue Jul 23, 2021
…amples

compiled into wasm binaries. Set up GHA to build wasm binaries, measure
file sizes, push results into benchmark dashboard .

Resolves ticket unicode-org#140.

Factor in review feedback.
gnrunge pushed a commit to gnrunge/icu4x that referenced this issue Jul 23, 2021
…amples

compiled into wasm binaries. Set up GHA to build wasm binaries, measure
file sizes, push results into benchmark dashboard .

Resolves ticket unicode-org#140.

Factor in review feedback.

Fix tidy clippy findings.
@sffc sffc closed this as completed in #871 Jul 29, 2021
sffc pushed a commit that referenced this issue Jul 29, 2021
…amples compiled into wasm binaries (#871)

Set up GHA to build wasm binaries, measure file sizes, push results into benchmark dashboard .

Resolves ticket #140.

Co-authored-by: Greg Tatum <gregtatum@users.noreply.github.com>

Co-authored-by: Greg Tatum <gregtatum@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-performance Area: Performance (CPU, Memory) C-test-infra Component: Integration test infrastructure S-medium Size: Less than a week (larger bug fix or enhancement) T-core Type: Required functionality
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants