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

Improve iridas_cube parsing speed #1630

Merged
merged 3 commits into from
May 16, 2022

Conversation

remia
Copy link
Collaborator

@remia remia commented Apr 12, 2022

While investigating long processor creation time (> 3.5sec), I noticed iridas_cube was really slow compared to spi3d for 3DLUT loading (on my system around 900ms and 250ms respectively for a 65x65x65 LUT). This brings it to similar performance.

Here is a benchmark using a 65x65x65 .cube 3DLUT, processor creation time (mean of 3 runs ocioperf --transform):

main pr
macOS 376 ms 216 ms
CentOS 7 927 ms 347 ms
Windows 10 1116 ms 715 ms

@remia
Copy link
Collaborator Author

remia commented Apr 12, 2022

Anyone knows why we use different code path for sscanf on Linux and Windows? Is there a reason (like compiler compatibility) that prevent us to use sscanf_s everywhere?

Signed-off-by: Rémi Achard <[email protected]>
Copy link
Member

@hodoulp hodoulp left a comment

Choose a reason for hiding this comment

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

Great performance improvement.

src/apputils/measure.h Show resolved Hide resolved
@remia
Copy link
Collaborator Author

remia commented Apr 27, 2022

I'll try to include Windows numbers shortly.

Edit: that's done.

Copy link
Collaborator

@doug-walker doug-walker left a comment

Choose a reason for hiding this comment

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

Thanks for the improvement Remi. Apologies for the delay in reviewing this.

@doug-walker doug-walker merged commit 44d8b85 into AcademySoftwareFoundation:main May 16, 2022
@remia remia deleted the iridas_cube_speed branch May 25, 2022 16:55
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.

3 participants