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 replacing double_conversion library with fast_float #2772

Conversation

Constellation
Copy link
Member

@Constellation Constellation commented Jul 27, 2022

178e072

Investigate replacing double_conversion library with fast_float
https://bugs.webkit.org/show_bug.cgi?id=221178

Reviewed by Sam Weinig.

Parsing double numbers becomes 2x faster.

                                  ToT                     Patched

    json-parse-double      230.0022+-0.0838     ^    111.1297+-0.0983        ^ definitely 2.0697x faster

Apple OSS approval for fast_float is OSS-4890.

* Source/WTF/WTF.xcodeproj/project.pbxproj:
* Source/WTF/wtf/CMakeLists.txt:
* Source/WTF/wtf/FastFloat.cpp: Added.
(WTF::parseDouble):
* Source/WTF/wtf/FastFloat.h: Added.
* Source/WTF/wtf/dtoa.h:
* Source/WTF/wtf/fast_float/LICENSE: Added.
* Source/WTF/wtf/fast_float/ascii_number.h: Added.
(fast_float::byteswap):
(fast_float::read_u64):
(fast_float::write_u64):
(fast_float::parse_eight_digits_unrolled):
* Source/WTF/wtf/fast_float/bigint.h: Added.
(fast_float::stackvec::stackvec):
(fast_float::bigint::bigint):
* Source/WTF/wtf/fast_float/decimal_to_binary.h: Added.
(fast_float::compute_product_approximation):
* Source/WTF/wtf/fast_float/digit_comparison.h: Added.
* Source/WTF/wtf/fast_float/fast_float.h: Added.
(fast_float::parse_options::parse_options):
* Source/WTF/wtf/fast_float/fast_table.h: Added.
* Source/WTF/wtf/fast_float/float_common.h: Added.
(fast_float::fastfloat_strncasecmp):
(fast_float::span::span):
(fast_float::value128::value128):
(fast_float::leading_zeroes):
(fast_float::emulu):
(fast_float::_umul128):
(fast_float::full_multiplication):
(fast_float::adjusted_mantissa::operator== const):
(fast_float::adjusted_mantissa::operator!= const):
(fast_float::binary_format<double>::mantissa_explicit_bits):
(fast_float::binary_format<float>::mantissa_explicit_bits):
(fast_float::binary_format<double>::max_exponent_round_to_even):
(fast_float::binary_format<float>::max_exponent_round_to_even):
(fast_float::binary_format<double>::min_exponent_round_to_even):
(fast_float::binary_format<float>::min_exponent_round_to_even):
(fast_float::binary_format<double>::minimum_exponent):
(fast_float::binary_format<float>::minimum_exponent):
(fast_float::binary_format<double>::infinite_power):
(fast_float::binary_format<float>::infinite_power):
(fast_float::binary_format<double>::sign_index):
(fast_float::binary_format<float>::sign_index):
(fast_float::binary_format<double>::min_exponent_fast_path):
(fast_float::binary_format<float>::min_exponent_fast_path):
(fast_float::binary_format<double>::max_exponent_fast_path):
(fast_float::binary_format<float>::max_exponent_fast_path):
(fast_float::binary_format<double>::max_mantissa_fast_path):
(fast_float::binary_format<float>::max_mantissa_fast_path):
(fast_float::binary_format<double>::exact_power_of_ten):
(fast_float::binary_format<float>::exact_power_of_ten):
(fast_float::binary_format<double>::largest_power_of_ten):
(fast_float::binary_format<float>::largest_power_of_ten):
(fast_float::binary_format<double>::smallest_power_of_ten):
(fast_float::binary_format<float>::smallest_power_of_ten):
(fast_float::binary_format<double>::max_digits):
(fast_float::binary_format<float>::max_digits):
(fast_float::binary_format<float>::exponent_mask):
(fast_float::binary_format<double>::exponent_mask):
(fast_float::binary_format<float>::mantissa_mask):
(fast_float::binary_format<double>::mantissa_mask):
(fast_float::binary_format<float>::hidden_bit_mask):
(fast_float::binary_format<double>::hidden_bit_mask):
(fast_float::to_float):
* Source/WTF/wtf/fast_float/parse_number.h: Added.
* Source/WTF/wtf/fast_float/simple_decimal_conversion.h: Added.
(fast_float::detail::trim):
(fast_float::detail::number_of_digits_decimal_left_shift):
(fast_float::detail::round):
(fast_float::detail::decimal_left_shift):
(fast_float::detail::decimal_right_shift):
(fast_float::compute_float):
(fast_float::parse_long_mantissa):

Canonical link: https://commits.webkit.org/257675@main

900ddd2

Misc iOS, tvOS & watchOS macOS Linux Windows
❌ πŸ§ͺ style βœ… πŸ›  ios βœ… πŸ›  mac βœ… πŸ›  wpe βœ… πŸ›  πŸ§ͺ win
βœ… πŸ›  ios-sim βœ… πŸ›  mac-AS-debug βœ… πŸ›  gtk βœ… πŸ›  wincairo
βœ… πŸ§ͺ webkitperl βœ… πŸ§ͺ ios-wk2 βœ… πŸ§ͺ api-mac βœ… πŸ§ͺ gtk-wk2
βœ… πŸ§ͺ api-ios βœ… πŸ§ͺ mac-wk1 ❌ πŸ§ͺ api-gtk
βœ… πŸ›  πŸ§ͺ jsc βœ… πŸ›  tv βœ… πŸ§ͺ mac-wk2 βœ… πŸ›  jsc-armv7
βœ… πŸ›  tv-sim βœ… πŸ§ͺ mac-AS-debug-wk2 βœ… πŸ§ͺ jsc-armv7-tests
βœ… πŸ›  πŸ§ͺ merge βœ… πŸ›  watch βœ… πŸ§ͺ mac-wk2-stress βœ… πŸ›  jsc-mips
βœ… πŸ›  watch-sim βœ… πŸ§ͺ jsc-mips-tests

@Constellation Constellation self-assigned this Jul 27, 2022
@Constellation Constellation force-pushed the eng/Investigate-replacing-double_conversion-library-with-fast_float branch from 3dd9123 to 9e41db1 Compare July 27, 2022 06:54
@webkit-early-warning-system webkit-early-warning-system added the merging-blocked Applied to prevent a change from being merged label Jul 27, 2022
@Constellation Constellation removed the merging-blocked Applied to prevent a change from being merged label Jul 27, 2022
@Constellation Constellation force-pushed the eng/Investigate-replacing-double_conversion-library-with-fast_float branch from 9e41db1 to e1d5f9f Compare July 27, 2022 07:00
@webkit-early-warning-system webkit-early-warning-system added the merging-blocked Applied to prevent a change from being merged label Jul 27, 2022
@Constellation Constellation removed the merging-blocked Applied to prevent a change from being merged label Jul 27, 2022
@Constellation Constellation force-pushed the eng/Investigate-replacing-double_conversion-library-with-fast_float branch from e1d5f9f to d1ebe50 Compare July 27, 2022 07:27
@webkit-early-warning-system webkit-early-warning-system added the merging-blocked Applied to prevent a change from being merged label Jul 27, 2022
@Constellation Constellation removed the merging-blocked Applied to prevent a change from being merged label Jul 27, 2022
@Constellation Constellation force-pushed the eng/Investigate-replacing-double_conversion-library-with-fast_float branch 2 times, most recently from 16c8fe0 to 07b0cb9 Compare July 27, 2022 09:30
@webkit-early-warning-system webkit-early-warning-system added the merging-blocked Applied to prevent a change from being merged label Jul 27, 2022
@Constellation Constellation removed the merging-blocked Applied to prevent a change from being merged label Jul 27, 2022
@Constellation Constellation force-pushed the eng/Investigate-replacing-double_conversion-library-with-fast_float branch 2 times, most recently from 32a2d64 to 4d7604d Compare July 29, 2022 21:20
Copy link
Contributor

@weinig weinig 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 following up on this!


namespace WTF {

WTF_EXPORT_PRIVATE double parseDouble(const LChar* string, size_t length, size_t& parsedLength);
Copy link
Contributor

Choose a reason for hiding this comment

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

I I think the way I would structured things would be to rename this file to ParseDouble.h and add all the ParseDouble header bits from dtoa.h here.

I would then name the file you have called FastFloat.cpp to ParseDoubleFastFloat.cpp.

And then, probably in a different change, rename dtoa.h to NumberToString.h and dtoa.cpp to NumberToStringDoubleConversion.cpp


#include <wtf/fast_float/float_common.h>

namespace fast_float {
Copy link
Contributor

Choose a reason for hiding this comment

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

For whatever reason, we wrapped all the double_conversion namespaces in an our namespace WTF. Not sure it makes a difference given this is all only included in one cpp file, but maybe for mangling reasons it would be a good idea?

@weinig
Copy link
Contributor

weinig commented Aug 1, 2022

Another good follow up to this would be removing the now unneeded parsing code from our copy of double_conversion.

@xeenon xeenon added WebCore Misc. For miscellaneous bugs in the WebCore framework (and not JavaScriptCore or WebKit). and removed WebKit Nightly Build labels Nov 21, 2022
@Constellation Constellation force-pushed the eng/Investigate-replacing-double_conversion-library-with-fast_float branch from 4d7604d to 498737e Compare December 2, 2022 04:38
@Constellation Constellation marked this pull request as ready for review December 2, 2022 04:40
@Constellation Constellation requested a review from a team as a code owner December 2, 2022 04:40
@Constellation Constellation force-pushed the eng/Investigate-replacing-double_conversion-library-with-fast_float branch from 498737e to b5e041d Compare December 2, 2022 04:45
@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Dec 2, 2022
@Constellation Constellation removed the merging-blocked Applied to prevent a change from being merged label Dec 2, 2022
@Constellation
Copy link
Member Author

ios-wk2 failures are flakiness ones.

@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Dec 3, 2022
@Constellation Constellation removed the merging-blocked Applied to prevent a change from being merged label Dec 7, 2022
@Constellation Constellation force-pushed the eng/Investigate-replacing-double_conversion-library-with-fast_float branch from b5e041d to 900ddd2 Compare December 7, 2022 06:01
@Constellation Constellation added the merge-queue Applied to send a pull request to merge-queue label Dec 10, 2022
https://bugs.webkit.org/show_bug.cgi?id=221178

Reviewed by Sam Weinig.

Parsing double numbers becomes 2x faster.

                                  ToT                     Patched

    json-parse-double      230.0022+-0.0838     ^    111.1297+-0.0983        ^ definitely 2.0697x faster

Apple OSS approval for fast_float is OSS-4890.

* Source/WTF/WTF.xcodeproj/project.pbxproj:
* Source/WTF/wtf/CMakeLists.txt:
* Source/WTF/wtf/FastFloat.cpp: Added.
(WTF::parseDouble):
* Source/WTF/wtf/FastFloat.h: Added.
* Source/WTF/wtf/dtoa.h:
* Source/WTF/wtf/fast_float/LICENSE: Added.
* Source/WTF/wtf/fast_float/ascii_number.h: Added.
(fast_float::byteswap):
(fast_float::read_u64):
(fast_float::write_u64):
(fast_float::parse_eight_digits_unrolled):
* Source/WTF/wtf/fast_float/bigint.h: Added.
(fast_float::stackvec::stackvec):
(fast_float::bigint::bigint):
* Source/WTF/wtf/fast_float/decimal_to_binary.h: Added.
(fast_float::compute_product_approximation):
* Source/WTF/wtf/fast_float/digit_comparison.h: Added.
* Source/WTF/wtf/fast_float/fast_float.h: Added.
(fast_float::parse_options::parse_options):
* Source/WTF/wtf/fast_float/fast_table.h: Added.
* Source/WTF/wtf/fast_float/float_common.h: Added.
(fast_float::fastfloat_strncasecmp):
(fast_float::span::span):
(fast_float::value128::value128):
(fast_float::leading_zeroes):
(fast_float::emulu):
(fast_float::_umul128):
(fast_float::full_multiplication):
(fast_float::adjusted_mantissa::operator== const):
(fast_float::adjusted_mantissa::operator!= const):
(fast_float::binary_format<double>::mantissa_explicit_bits):
(fast_float::binary_format<float>::mantissa_explicit_bits):
(fast_float::binary_format<double>::max_exponent_round_to_even):
(fast_float::binary_format<float>::max_exponent_round_to_even):
(fast_float::binary_format<double>::min_exponent_round_to_even):
(fast_float::binary_format<float>::min_exponent_round_to_even):
(fast_float::binary_format<double>::minimum_exponent):
(fast_float::binary_format<float>::minimum_exponent):
(fast_float::binary_format<double>::infinite_power):
(fast_float::binary_format<float>::infinite_power):
(fast_float::binary_format<double>::sign_index):
(fast_float::binary_format<float>::sign_index):
(fast_float::binary_format<double>::min_exponent_fast_path):
(fast_float::binary_format<float>::min_exponent_fast_path):
(fast_float::binary_format<double>::max_exponent_fast_path):
(fast_float::binary_format<float>::max_exponent_fast_path):
(fast_float::binary_format<double>::max_mantissa_fast_path):
(fast_float::binary_format<float>::max_mantissa_fast_path):
(fast_float::binary_format<double>::exact_power_of_ten):
(fast_float::binary_format<float>::exact_power_of_ten):
(fast_float::binary_format<double>::largest_power_of_ten):
(fast_float::binary_format<float>::largest_power_of_ten):
(fast_float::binary_format<double>::smallest_power_of_ten):
(fast_float::binary_format<float>::smallest_power_of_ten):
(fast_float::binary_format<double>::max_digits):
(fast_float::binary_format<float>::max_digits):
(fast_float::binary_format<float>::exponent_mask):
(fast_float::binary_format<double>::exponent_mask):
(fast_float::binary_format<float>::mantissa_mask):
(fast_float::binary_format<double>::mantissa_mask):
(fast_float::binary_format<float>::hidden_bit_mask):
(fast_float::binary_format<double>::hidden_bit_mask):
(fast_float::to_float):
* Source/WTF/wtf/fast_float/parse_number.h: Added.
* Source/WTF/wtf/fast_float/simple_decimal_conversion.h: Added.
(fast_float::detail::trim):
(fast_float::detail::number_of_digits_decimal_left_shift):
(fast_float::detail::round):
(fast_float::detail::decimal_left_shift):
(fast_float::detail::decimal_right_shift):
(fast_float::compute_float):
(fast_float::parse_long_mantissa):

Canonical link: https://commits.webkit.org/257675@main
@webkit-early-warning-system webkit-early-warning-system force-pushed the eng/Investigate-replacing-double_conversion-library-with-fast_float branch from 900ddd2 to 178e072 Compare December 10, 2022 06:56
@webkit-commit-queue
Copy link
Collaborator

Committed 257675@main (178e072): https://commits.webkit.org/257675@main

Reviewed commits have been landed. Closing PR #2772 and removing active labels.

@webkit-commit-queue webkit-commit-queue removed the merge-queue Applied to send a pull request to merge-queue label Dec 10, 2022
@webkit-early-warning-system webkit-early-warning-system merged commit 178e072 into WebKit:main Dec 10, 2022
@Constellation Constellation deleted the eng/Investigate-replacing-double_conversion-library-with-fast_float branch December 12, 2022 00:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
WebCore Misc. For miscellaneous bugs in the WebCore framework (and not JavaScriptCore or WebKit).
Projects
None yet
6 participants