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

Use radsort for Transparent2d PhaseItem sorting #9882

Merged
merged 6 commits into from
Sep 21, 2023

Conversation

rparrett
Copy link
Contributor

@rparrett rparrett commented Sep 21, 2023

Objective

Fix a performance regression in the "bevy vs pixi" benchmark.

This benchmark seems to have a slightly pathological distribution of z values -- Sprites are spawned with a random z value with a child sprite at f32::EPSILON relative to the parent.

See discussion here: #8100 (comment)

Solution

Use radsort for sorting Transparent2d PhaseItems.

Use random z values in bevymark to stress the phase sort. Add an --ordered-z option to bevymark that uses the old behavior.

Benchmarks

mac m1 max

benchmark fps before fps after diff
bevymark --waves 120 --per-wave 1000 --random-z 42.16 47.06 🟩 +11.6%
bevymark --waves 120 --per-wave 1000 52.50 52.29 🟥 -0.4%
bevymark --waves 120 --per-wave 1000 --mode mesh2d --random-z 9.64 10.24 🟩 +6.2%
bevymark --waves 120 --per-wave 1000 --mode mesh2d 15.83 15.59 🟥 -1.5%
bevy-vs-pixi 39.71 59.88 🟩 +50.1%

Discussion

It's possible that TransparentUi should also change. We could probably use slice::sort_unstable_by_key with the current sort key though, as its items are always sorted and unique. I'd prefer to follow up later to look into that.

Here's a survey of sorts used by other PhaseItems

slice::sort_by_key

Transparent2d, TransparentUi

radsort

Opaque3d, AlphaMask3d, Transparent3d, Opaque3dPrepass, AlphaMask3dPrepass, Shadow

I also tried slice::sort_unstable_by_key with a compound sort key including Entity, but it didn't seem as promising and I didn't test it as thoroughly.

@rparrett rparrett requested a review from superdump September 21, 2023 01:04
@rparrett rparrett added A-Rendering Drawing game state to the screen C-Performance A change motivated by improving speed, memory usage or compile times labels Sep 21, 2023
Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
Copy link
Contributor

@superdump superdump left a comment

Choose a reason for hiding this comment

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

I think this is fine as-is. But a couple of comments anyway.

examples/stress_tests/bevymark.rs Outdated Show resolved Hide resolved
crates/bevy_core_pipeline/src/core_2d/mod.rs Show resolved Hide resolved
@rparrett rparrett changed the title Use radsort for Transparent2d PhaseItems sorting Use radsort for Transparent2d PhaseItem sorting Sep 21, 2023
rparrett and others added 2 commits September 21, 2023 09:22
Co-authored-by: Robert Swain <robert.swain@gmail.com>
@superdump superdump added this pull request to the merge queue Sep 21, 2023
Merged via the queue into bevyengine:main with commit bdb0634 Sep 21, 2023
24 of 25 checks passed
@cart cart mentioned this pull request Oct 13, 2023
43 tasks
rdrpenguin04 pushed a commit to rdrpenguin04/bevy that referenced this pull request Jan 9, 2024
# Objective

Fix a performance regression in the "[bevy vs
pixi](https://github.com/SUPERCILEX/bevy-vs-pixi)" benchmark.

This benchmark seems to have a slightly pathological distribution of `z`
values -- Sprites are spawned with a random `z` value with a child
sprite at `f32::EPSILON` relative to the parent.

See discussion here:
bevyengine#8100 (comment)

## Solution

Use `radsort` for sorting `Transparent2d` `PhaseItem`s.

Use random `z` values in bevymark to stress the phase sort. Add an
`--ordered-z` option to `bevymark` that uses the old behavior.

## Benchmarks

mac m1 max

| benchmark | fps before | fps after | diff |
| - | - | - | - |
| bevymark --waves 120 --per-wave 1000 --random-z | 42.16 | 47.06 | 🟩
+11.6% |
| bevymark --waves 120 --per-wave 1000 | 52.50 | 52.29 | 🟥 -0.4% |
| bevymark --waves 120 --per-wave 1000 --mode mesh2d --random-z | 9.64 |
10.24 | 🟩 +6.2% |
| bevymark --waves 120 --per-wave 1000 --mode mesh2d | 15.83 | 15.59 | 🟥
-1.5% |
| bevy-vs-pixi | 39.71 | 59.88 | 🟩 +50.1% |

## Discussion

It's possible that `TransparentUi` should also change. We could probably
use `slice::sort_unstable_by_key` with the current sort key though, as
its items are always sorted and unique. I'd prefer to follow up later to
look into that.

Here's a survey of sorts used by other `PhaseItem`s

#### slice::sort_by_key
`Transparent2d`, `TransparentUi`

#### radsort
`Opaque3d`, `AlphaMask3d`, `Transparent3d`, `Opaque3dPrepass`,
`AlphaMask3dPrepass`, `Shadow`

I also tried `slice::sort_unstable_by_key` with a compound sort key
including `Entity`, but it didn't seem as promising and I didn't test it
as thoroughly.

---------

Co-authored-by: Alice Cecile <alice.i.cecile@gmail.com>
Co-authored-by: Robert Swain <robert.swain@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Rendering Drawing game state to the screen C-Performance A change motivated by improving speed, memory usage or compile times
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants