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 sprite sorting benchmark #8458

Merged
merged 7 commits into from
Jan 22, 2019
Merged

Conversation

janisozaur
Copy link
Member

See #8442

@IntelOrca
Copy link
Contributor

Why can't the benchmark just be run on the actual game?

@janisozaur
Copy link
Member Author

It probably could, that also gave me the idea that I could extract paint session data from a park passed in an argument, which solves the long compilation time and the need to extract it beforehand.

I'll work on that.

@janisozaur
Copy link
Member Author

I purposely implemented the benchmark as a single file initially so it is easier to experiment with. Right now there is no verification of the order of sorted sprites is valid (same as some predefined order), but with screenshot command perhaps I wouldn't need to implement that.

@ZehMatt
Copy link
Member

ZehMatt commented Dec 16, 2018

I think it would be slightly better to move the sorter out into its own and let the benchmark use that code, otherwise this adds duplicate code.

@IntelOrca
Copy link
Contributor

The problem is that that test input data will become out of date very quickly particularly if the structs get changed. It would be much better (easier to maintain) if it just benchmarked via calling the paint function with a loaded saved game.

@ZehMatt
Copy link
Member

ZehMatt commented Dec 17, 2018

I agree with @IntelOrca here, it would probably be better to have a benchmark like I described here: #6339 (comment)

@janisozaur
Copy link
Member Author

janisozaur commented Jan 6, 2019

This was significantly improved, it will now load the park, automatically extract the needed data and time the sprite sorting function.

This is not done yet, but should be actually usable now.

Things to improve:

  • The park to load is hardcoded to data.sv6
  • Make sure to actually load park only once
  • Consider integration with main binary
  • Current approach is to pass a pointer to std::vector<paint_struct> to viewport paint functions. Perhaps a better approach would be to pass a function object/lambda?
  • Consider adding some baseline
  • Remove the test/tests/SpriteBenchmarkData.inc, now obsolete
  • Hook up CI
  • Update the comment
  • Implement some benchmark performance improvements
  • Make a call to state.setItemsProcessed() to get nice performance labels

To launch: compile, select some park, name it data.sv6 in current directory, start the test. It can take a looong time to finish, I suggest running it this way:

$ ./test_sprite_sort_benchmark --benchmark_min_time=0.001
<some spam here>
----------------------------------------------------------------
Benchmark                         Time           CPU Iterations
----------------------------------------------------------------
BM_paint_session_arrange      14417 ns      11830 ns        114

@ZehMatt
Copy link
Member

ZehMatt commented Jan 7, 2019

I have somewhat mixed feelings about this benchmark. If we decide to go for a better painting structure the whole code may become obsolete and I'm not sure you can actually improve the current sorting method to something that has a measurable increase, so measuring the current sorter is somewhat pointless, I would rather know how much time was spent to a build the entire frame and how long each component took. I'm just worried that the sorting code will either be dropped entirely or that no one comes up with a much faster version.

@janisozaur
Copy link
Member Author

We can remove the benchmark code if its structure changes to the point where it can longer be supported. Before that time comes though I'd like to merge this, I would like to try out some things with the sprite sorter. Additionally, it would be easier to implement more benchmarks once this lands for other parts of code.

@janisozaur
Copy link
Member Author

Thing still needed from the list above and others:

  • Hook up CI? Would likely require objects, but it builds now, which satisfies me.
  • Make sure to actually load park only once
  • Correct parsing of arguments for benchmark library
  • Un-hardcode the park being loaded.

@IntelOrca
Copy link
Contributor

You can also update https://github.com/OpenRCT2/OpenRCT2/wiki/Benchmarking-&-stress-testing-OpenRCT2 afterwards.

@janisozaur janisozaur force-pushed the benchmark branch 4 times, most recently from 6464eda to 68235c9 Compare January 21, 2019 22:05
@janisozaur
Copy link
Member Author

This should be considered done now.

Make sure google benchmark is installed, build, then:

$ ./openrct2 benchspritesort ~/.config/OpenRCT2/save/dome-roof-on_zoom1.sv6 --benchmark_min_time=0.001
INFO[../src/openrct2/cmdline/BenchSpriteSort.cpp:74 (extract_paint_session)]: Starting...
INFO[../src/openrct2/cmdline/BenchSpriteSort.cpp:128 (extract_paint_session)]: Obtaining sprite data...
INFO[../src/openrct2/cmdline/BenchSpriteSort.cpp:134 (extract_paint_session)]: Got 242 paint sessions.
2019-01-21 23:08:31
Run on (8 X 4400 MHz CPU s)
CPU Caches:
  L1 Data 32K (x4)
  L1 Instruction 32K (x4)
  L2 Unified 256K (x4)
  L3 Unified 8192K (x1)
-----------------------------------------------------------------------------------------------------
Benchmark                                                              Time           CPU Iterations
-----------------------------------------------------------------------------------------------------
baseline                                                             437 ns        376 ns       2680   2.53875M items/s
/home/janisozaur/.config/OpenRCT2/save/dome-roof-on_zoom1.sv6       8450 ns       6784 ns        194   34.0183M items/s

@codecov-io

This comment has been minimized.

@janisozaur janisozaur merged commit bca8870 into OpenRCT2:develop Jan 22, 2019
janisozaur added a commit that referenced this pull request Mar 13, 2019
- Feature: [#4418] Allow steep slopes on the side-friction roller coaster.
- Feature: [#7726] Add shortcut to advance one tick.
- Feature: [#7956, #7964] Add sprite font glyphs for Hungarian and some Czech letters.
- Feature: [#7971] Toolbox option to open custom content folder.
- Feature: [#7980] Allow data path for RCT1 to be specified by a command line argument.
- Feature: [#8073] Auto-upload minidumps to backtrace.io (optional, MSVC/Windows only)
- Feature: [#8078] Add save_park command to in-game console.
- Feature: [#8080] New console variable "current_rotation" to get or set view rotation.
- Feature: [#8098] Glyph for Russian rouble sign.
- Feature: [#8099] Add Powered Launch mode to Inverted RC (for RCT1 parity).
- Feature: [#8190] Allow building footpaths on 'corner down' terrain.
- Feature: [#8191] Allow building on-ride photos and water S-bends on the Water Coaster.
- Feature: [#8259] Add say command to in-game console.
- Feature: [#8374] Add replay system.
- Feature: [#8377] Add option to adjust amount of autosaves to keep.
- Feature: [#8458] Add sprite sorting benchmark.
- Feature: [#8583] Add boosters to water coaster.
- Feature: [#8648] Add optional chat button to top toolbar in multiplayer games.
- Feature: [#8652] Add network window including a graph for data usage visualisation.
- Feature: [#8670] Add ability to download missing objects when loading a park.
- Change: [#7961] Add new object types: station, terrain surface, and terrain edge.
- Change: [#8222] The climate setting has been moved from objective options to scenario options.
- Change: [#8718] Allow TARMAC object to be removed when running the `remove_unused_objects` command.
- Change: [#8718] No longer require the generic scenery groups and tarmac footpath to be checked when creating a scenario.
- Change: [#8734] Disable kick button in multiplayer window when unable to use it.
- Fix: [#3832] Changing the colour scheme of track pieces does not work in multiplayer.
- Fix: [#4094] Coasters with long flat-to-steep pieces offer them in diagonal mode (original bug).
- Fix: [#5684] Player list can desync between clients and server and can crash.
- Fix: [#6191] OpenRCT2 fails to run when the path has an emoji in it.
- Fix: [#7439] Placement messages have mixed strings
- Fix: [#7473] Disabling sound effects also disables "Disable audio on focus loss".
- Fix: [#7536] Android builds fail to start.
- Fix: [#7689] Deleting 0-tile maze gives a MONEY32_UNDEFINED (negative) refund.
- Fix: [#7828] Copied entrances and exits stay when demolishing ride.
- Fix: [#7945] Client IP address is logged as `(null)` in server logs.
- Fix: [#7952] Performance drop caused by code refactor.
- Fix: [#7954] Key validation fails on Windows due to non-ASCII user / player name.
- Fix: [#7975] Inspection flag not cleared for rides which are set to never be inspected (original bug).
- Fix: [#7985] Giant Screenshot ignores 'Map rendering' settings.
- Fix: [#7987] Broken track designs increase money by MONEY32_UNDEFINED.
- Fix: [#7991] Scenery and footpaths on Construction Rights tiles can be deleted using Clear Scenery.
- Fix: [#8034] Vanilla sprites are broken when making screenshots from command line.
- Fix: [#8045] Crash when switching between languages.
- Fix: [#8062] In multiplayer warnings for unstable cheats are shown when disabling them.
- Fix: [#8090] Maze designs saved incorrectly.
- Fix: [#8101] Title sequences window flashes after opening.
- Fix: [#8120] Crash trying to place peep spawn outside of map.
- Fix: [#8121] Crash Renaming park with server logging enabled.
- Fix: [#8139] Buying land costs money when the game is in "no money" mode.
- Fix: [#8141] Attempting to build entrance/exit on station 2 does not work.
- Fix: [#8142] Reliability of mazes and crooked houses can go below 100%.
- Fix: [#8187] Cannot set land ownership over ride entrances or exits in sandbox mode.
- Fix: [#8200] Incorrect behaviour when removing entrances and exits that are on the same tile.
- Fix: [#8204] Crash when tile element has no surface elements.
- Fix: [#8264] Rides and scenery placeable outside of map with ZC and Sandbox mode enabled.
- Fix: [#8335] Rides with arbitrary ride types can crash the game when they break down.
- Fix: [#8358] Infinite loop when changing vehicle count on stopped ride.
- Fix: [#8402] Crash closing a window in some cases.
- Fix: [#8431] Crash when game action logging is enabled.
- Fix: [#8433] Crash if master server response is not valid JSON.
- Fix: [#8434] Crash if curl_easy_init fails.
- Fix: [#8443] Crash when selecting the current vehicle for ride that has none available.
- Fix: [#8456] Junior booster track piece doesn't connect properly.
- Fix: [#8464] Crash on game shutdown.
- Fix: [#8469] Crash modifying colour on hacked rides.
- Fix: [#8508] Underground roto-drop is not going up.
- Fix: [#8555] Multiplayer window text limits are not computed properly.
- Fix: [#8572] Steel Twister track pieces ID 64 and 65 drawn incorrectly.
- Fix: [#8585] Part of track missing on air powered vertical coaster.
- Fix: [#8588] Guest list scrolling breaks above ~2000 guests.
- Fix: [#8591] Game loop does not run at a consistent tick rate of 40 Hz.
- Fix: [#8647] Marketing campaigns check for entry fees below £1 (original bug).
- Fix: [#8653] Crash when peeps attempt to enter a ride with no vehicles.
- Fix: [#8720] Desync due to boats colliding with ghost pieces.
- Fix: [#8739] Savegame from original game crashes when cruising through map.
- Fix: [#8742] Access violation in vehicle_update_sound_params.
- Fix: [#8804] Raising water shows money effect at the bottom rather than new height.
- Fix: [#8811] Some fields in the sv6 save file not being copied correctly.
- Fix: [#8824] Invalid read in footpath_chain_ride_queue.
- Improved: [#2940] Allow mouse-dragging to set patrol area (Singleplayer only).
- Improved: [#7730] Draw extreme vertical and lateral Gs red in the ride window's graph tab.
- Improved: [#7930] Automatically create folders for custom content.
- Improved: [#7980] Show the full path of the scenario in the scenario select window.
- Improved: [#7993] Allow assigning a keyboard shortcut for opening the tile inspector.
- Improved: [#8107] Support Discord release of RCT2.
- Improved: [#8491] Highlight entrance and exit with different colours in track design previews.
- Improved: Almost completely new Hungarian translation.
- Removed: [#7929] Support for scenario text objects.
@janisozaur janisozaur deleted the benchmark branch March 30, 2024 18:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants