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

Investigate integrating with fast_float #1069

Open
madolson opened this issue Sep 24, 2024 · 7 comments
Open

Investigate integrating with fast_float #1069

madolson opened this issue Sep 24, 2024 · 7 comments
Assignees
Labels
help wanted External contributions would be appreciated

Comments

@madolson
Copy link
Member

I've seen a couple of posts recently about how both dragonflyDB and Redis integrated with https://github.com/fastfloat/fast_float to improve parsing of doubles. We use strtod (exposed as getDoubleFromObject) in sorted set computation. The library was implemented in C++, but we should be able to extract out the needed code that solves our specific use case.

@madolson madolson added the help wanted External contributions would be appreciated label Sep 24, 2024
@swaingotnochill
Copy link

Hi, I would like to work on this one. Additionally, this will be my first time contributing to Valkey so it would be great if someone could review my work alongside.

@PingXie
Copy link
Member

PingXie commented Sep 29, 2024

go for it, @swaingotnochill. You can tag us in your PR when it is ready for review.

@swaingotnochill
Copy link

Thanks. I will start working on it and add you guys when I have a draft PR ready.

@swaingotnochill
Copy link

swaingotnochill commented Oct 2, 2024

@madolson @PingXie I looked at the fast_float, they have scripts to get it in a single header file to use. Then it's possible to create an interface to expose specific functions just like other dependencies in "deps" folder. And looking at Redis code, they have done something similar. Is the plan to replace the strtod in the sorted_set implementation Or all of the occurrences of strtod? (From the benchmarks of fast_float, its 5 times faster than strtod for parsing random floating numbers)

@parthpatel
Copy link
Member

How would you expose an interface for a function with templating? Are you proposing simply initializing a version of that function for a type and exposing an interface for it?

template <typename T, typename UC = char,
          typename = FASTFLOAT_ENABLE_IF(is_supported_float_type<T>())>
FASTFLOAT_CONSTEXPR20 from_chars_result_t<UC>
from_chars(UC const *first, UC const *last, T &value,
           chars_format fmt = chars_format::general) noexcept;

@swaingotnochill
Copy link

How would you expose an interface for a function with templating? Are you proposing simply initializing a version of that function for a type and exposing an interface for it?

template <typename T, typename UC = char,
          typename = FASTFLOAT_ENABLE_IF(is_supported_float_type<T>())>
FASTFLOAT_CONSTEXPR20 from_chars_result_t<UC>
from_chars(UC const *first, UC const *last, T &value,
           chars_format fmt = chars_format::general) noexcept;

Something like this: https://stackoverflow.com/questions/2744181/how-to-call-c-function-from-c (look at the top answer).
Basically, create a wrapper over fast_float to use it in C.

@madolson
Copy link
Member Author

madolson commented Oct 4, 2024

@madolson @PingXie I looked at the fast_float, they have scripts to get it in a single header file to use. Then it's possible to create an interface to expose specific functions just like other dependencies in "deps" folder. And looking at Redis code, they have done something similar. Is the plan to replace the strtod in the sorted_set implementation Or all of the occurrences of strtod? (From the benchmarks of fast_float, its 5 times faster than strtod for parsing random floating numbers)

I would say the numeric parsing in sorted_sets is the most important, but I see no reason to not replace all of the usage of strod.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted External contributions would be appreciated
Projects
None yet
Development

No branches or pull requests

4 participants