-
Notifications
You must be signed in to change notification settings - Fork 630
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 benchmarking mode to exrmetrics #1956
base: main
Are you sure you want to change the base?
Add benchmarking mode to exrmetrics #1956
Conversation
Thanks for this work! This PR is basically adding a completely different mode to exrmetrics. The main features it adds are CSV output, which is nice for scripted systems, and running multiple iterations, but there are a bunch of limitations and differences between the two modes which give me pause. As far as I can tell:
Also, looking at Perhaps we could refactor so there's a single code path between the two modes, and have basically a bunch of flags that switch individual features from the current behavior of exrmetrics to the exrbench behavior. So exrbench as it currently is implemented would be enabled with flags like this: I might also have made the top level loop the pixel type, so main() calls the exrbench function multiple times for each pixel type, which then iterates through each compression type, rather than having both float and half types within exrbench. That reduces the amount of memory required as well as making the code simpler. That would also make it possible to specify a single file for all writing. That said, maybe there should just be 'original' and 'half'. exrmetrics doesn't currently offer a test where it converts halfs to floats because I thought that was misleading: the data is still quantized to half, so file size, and potentially performance, isn't a true representation of "real" floating point data. One completely independent change I was considering was being able to test all parts of a multipart file. That would also allow exrmetrics to convert a file between compression types without loss of data. It would make sense to do that as part of this refactor. (A --convert flag would be equivalent to @peterurbanec if you are keen to make some changes along those lines then go ahead. Otherwise I'm happy to make a start on that myself when I can. |
6f2cc3b
to
d8c737e
Compare
This is designed to help with performance regression testing and has been used to gather data for issue AcademySoftwareFoundation#1915. Signed-off-by: Peter Urbanec <[email protected]>
d8c737e
to
bf23dee
Compare
I force-pushed some updates to address failing CI test cases. @peterhillman Thank you for the very comprehensive write up. There are a lot of great ideas there. My motivation was to create a quick and simple test case for reproducing a slow down that I reported in issue #1915 . I also have a backport of these changes for OpenEXR 3.2.1, again, motivated by the need to compare performance between releases to provide sufficient data for #1915. Unfortunately, I do not have the time to do much more development on the benchmarking mode. I was hoping that by adding an initial implementation, I would get the ball rolling on making performance regression testing a part of the OpenEXR development cycle. I always anticipated that people more familiar with OpenEXR workings would be able to enhance the tests and integrate them into CI pipelines. IMHO, it might be worthwhile to start by merging this simple implementation and make enhancements over time. The current implementation is sufficient for getting a rough idea of OpenEXR performance. |
This is designed to help with performance regression testing and has been used to gather data for issue #1915.