search

zesterer / euc

A software rendering crate that lets you write shaders with Rust
Apache License 2.0
294 stars 18 forks source link

Added a benchmark using criterion.rs #4

Closed sunjay closed 5 years ago

sunjay commented 5 years ago

This PR adds a benchmark to measure the time it takes to render the teapot example at various resolutions. The benchmark uses criterion.rs and works on stable. Read the guide for that library for more information/usage.

This is very useful when testing potential performance enhancements. As an example, I tried to optimize the triangle rasterizer by removing the branching on the depth strategy on every iteration. The depth strategy doesn't change during the rasterization process, so we can use a closure (which will get inlined) to do this instead. The changes and results are here. While we got some significant improvements for a few resolutions, it was not an improvement across the board. Thus, I did not submit that as part of this PR.

image

I also snuck in a second change to set opt-level = 3 in Cargo.toml. This just saves us from having to pass in --release all the time. Users of this library will still have to do that. It's just a quality of life improvement for us during development.

P.S. I think you may have some unpushed changes. The Cargo.toml file still lists version 0.2. I can rebase this branch once you've pushed.

sunjay commented 5 years ago

One of the reasons I did this was because I wanted to try and add rayon as we discussed in #1. My attempt showed me exactly what you meant in your comment and I now understand all the problems with the depth buffer. This is definitely harder than I thought it would be!

zesterer commented 5 years ago

Thanks! Sorry about the late reply, I've had a busy few days. These are good changes, for sure. I might work on getting Travis set up to do this sort of stuff automatically in the future.

zesterer commented 5 years ago

One thing I would say is that we should remove file loading from the benchmark to ensure that I/O doesn't mess with the numbers.

sunjay commented 5 years ago

One thing I would say is that we should remove file loading from the benchmark to ensure that I/O doesn't mess with the numbers.

Yup! The only thing I'm measuring is the time it takes to run draw. Notice where iter is called.

zesterer commented 5 years ago

Ah, apologies. I only glanced at the code, and it seems that just glancing didn't pay off.

sunjay commented 5 years ago

No problem! The benchmark can definitely be improved in the future. I think this is a good start though.

Good idea about adding Travis! We should think about adding tests. :)

Thanks for merging my PR!