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

Remove lazy-static and make FCACHE a proper const #211

Merged
merged 1 commit into from
Apr 22, 2024

Conversation

anergictcell
Copy link
Contributor

This minor change removes the lazy_static dependency by making the FCACHE a const.

@YeungOnion
Copy link
Contributor

Dropping another dependency would be nice. Do you know if this will still be static or will it work like inlining?

@anergictcell
Copy link
Contributor Author

anergictcell commented Apr 17, 2024 via email

@FreezyLemon
Copy link
Contributor

FreezyLemon commented Apr 17, 2024

A const is not static in the C/C++ sense of the word. You should expect inlining when you use a const variable.

That said, you don't have to switch to const. static will mostly work the same way (just declare it as static FCACHE) and will instead put it into some memory location in the binary instead of inlining.

const is generally preferable because it allows for more optimizations, but if binary size is a concern or you are otherwise sure you want a static, it's no problem to switch.

@YeungOnion
Copy link
Contributor

Thanks for commenting on this one @FreezyLemon, I didn't know there's more chance of optimizations, so I learned something new.

I think FCACHE appears only twice in the entire crate. An additional [f64; 170] is a lot smaller than the crate. Seems like a great idea to change to const. Will merge!

@YeungOnion YeungOnion merged commit d29d020 into statrs-dev:master Apr 22, 2024
2 checks passed
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