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

tests: Add criterion for basic benchmarking #6071

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

Herschel
Copy link
Member

@Herschel Herschel commented Jan 16, 2022

Add criterion with some basic benchmark SWFs, and refactor some of the test code to support this.

criterion seems geared towards smaller benchmarks, and these benchmarks are already a little chunky -- it's possible something like bencher might be more suitable.

TODO, probably future PRs:

  • Benchmark AVM more directly
    • It'd be nice to benchmark AVM code directly instead of running the entire frame loop & player machinery.
    • Trickier with AVM1 because it's so coupled to the timeline and displaylist.
    • This should be good enough for comparisons for now.
  • TestBuilder could become a more general PlayerBuilder available in core.
  • Get rid of the giant list of tests in regression_tests.rs and instead generate tests via directory listing similar to the benchmarks
    • Tests that need a specific config (approx, frame count, etc.) can be specified, otherwise can be omitted.
    • I think this needs to be done via procmacro or build.rs for tests.
  • Add some AVM2 and general playback benchmarks.

@Herschel Herschel force-pushed the criterion branch 2 times, most recently from 1383ed3 to 17246ea Compare January 16, 2022 07:09
@adrian17
Copy link
Collaborator

Tests that need a specific config (approx, frame count, etc.)

Could also put test-specific configs as special comments in output.txt. (I'm thinking of something like LLVM's tests.)

But also... are there cases where we want different frame count than the number of frames in the test SWF itself?

@adrian17
Copy link
Collaborator

Also, let's maybe rename the benches to make it explicit they test AVM1?

@Herschel
Copy link
Member Author

Herschel commented Jan 16, 2022

are there cases where we want different frame count than the number of frames in the test SWF itself?

Quite a few, if you look through the tests (gotoAndStops, looping anims, timers, ...). But using the # of frames of the main timeline is a sensible default that I'd like to add.

Could also put test-specific configs as special comments in output.txt. (I'm thinking of something like LLVM's tests.)

I started working towards a more general test + SWF compiler over here, and I figure these settings can be a part of that syntax. But in the meantime doing some YAML front matter in output.txt would work nicely.

@Herschel
Copy link
Member Author

Herschel commented Jan 17, 2022

Currently the results seem to vary quite a lot across runs, so I'm going to try to narrow it down to run the AVM code more directly.

@adrian17
Copy link
Collaborator

adrian17 commented Jan 17, 2022

large_array_init feels small enough that it might be beneficial to even wrap the array creation in an ActionScript loop, to minimize relative startup overhead compared to actual AS runtime.

@Herschel Herschel force-pushed the criterion branch 2 times, most recently from 5bf7a1d to 8d242a4 Compare January 18, 2022 19:30
Remove `Player::add_external_interface` now that these can be
specified at constructor time.
@zamazan4ik
Copy link

Hey, @Herschel! What is the status of this PR?

@torokati44 torokati44 mentioned this pull request May 21, 2023
@danielhjacobs danielhjacobs added A-tests Area: Tests & Test Framework T-feature Type: New Feature (that Flash doesn't have) T-perf T-perf Type: Performance Improvements and removed T-feature Type: New Feature (that Flash doesn't have) T-perf labels Sep 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-tests Area: Tests & Test Framework T-perf Type: Performance Improvements
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants