ticket:13672 Enabling double_conversion StringToDoubleConverter and using it in DecimalQuantity's t…

ticket:13672 Enabling double_conversion StringToDoubleConverter and using it in DecimalQuantity's toDouble() function.

[sffc, 2018-03-30 07:11:51]

https://codereview.appspot.com/340990043/diff/1/icu4c/source/i18n/number_deci...
File icu4c/source/i18n/number_decimalquantity.cpp (right):

https://codereview.appspot.com/340990043/diff/1/icu4c/source/i18n/number_deci...
icu4c/source/i18n/number_decimalquantity.cpp:1013: UnicodeString
DecimalQuantity::toNumberString() const {
Since my change puts this function in the critical path of parsing, I spent a
little time trying to optimize it.  I think UnicodeString append operations are
very fast.  Does this seem reasonable?  Can you think of a better way to do
this, go from BCD to a string of the form "###E###"?

[andy.heninger, 2018-04-03 18:32:52]

Note: the corresponding ICU ticket is not in reviewing state.

https://codereview.appspot.com/340990043/diff/1/icu4c/source/i18n/number_deci...
File icu4c/source/i18n/number_decimalquantity.cpp (right):

https://codereview.appspot.com/340990043/diff/1/icu4c/source/i18n/number_deci...
icu4c/source/i18n/number_decimalquantity.cpp:1013: UnicodeString
DecimalQuantity::toNumberString() const {
On 2018/03/30 07:11:51, sffc wrote:
> Since my change puts this function in the critical path of parsing, I spent a
> little time trying to optimize it.  I think UnicodeString append operations
are
> very fast.  Does this seem reasonable?  Can you think of a better way to do
> this, go from BCD to a string of the form "###E###"?

This seems good to me. You could construct the UnicodeString with an estimate of
the needed capacity, but I don't think it's worth the trouble - the stack buffer
holds 27, which statistically covers everything that matters for performance.

https://codereview.appspot.com/340990043/diff/1/icu4c/source/i18n/number_deci...
icu4c/source/i18n/number_decimalquantity.cpp:1029: result.insert(insertIndex,
u'0' + (_scale % 10));
there's a stdlib.h function div() that will give you the quotient and remainder
in a single operation. (CPU divide instructions give both).

https://codereview.appspot.com/340990043/diff/1/icu4c/source/test/intltest/nu...
File icu4c/source/test/intltest/numbertest_decimalquantity.cpp (right):

https://codereview.appspot.com/340990043/diff/1/icu4c/source/test/intltest/nu...
icu4c/source/test/intltest/numbertest_decimalquantity.cpp:298: void
DecimalQuantityTest::testToDouble() {
Does the new double implementation work with the old IBM mainframe double
format?

https://codereview.appspot.com/340990043/diff/1/icu4c/source/test/intltest/nu...
icu4c/source/test/intltest/numbertest_decimalquantity.cpp:304: { "-3.142E-271",
-3.142e-271 } };
What is the significance of these specific numbers?

[sffc, 2018-04-05 23:44:32]

Fixes committed in r41200 and r41201.

https://codereview.appspot.com/340990043/diff/1/icu4c/source/i18n/number_deci...
File icu4c/source/i18n/number_decimalquantity.cpp (right):

https://codereview.appspot.com/340990043/diff/1/icu4c/source/i18n/number_deci...
icu4c/source/i18n/number_decimalquantity.cpp:1029: result.insert(insertIndex,
u'0' + (_scale % 10));
On 2018/04/03 18:32:51, andy.heninger wrote:
> there's a stdlib.h function div() that will give you the quotient and
remainder
> in a single operation. (CPU divide instructions give both).

Done.

https://codereview.appspot.com/340990043/diff/1/icu4c/source/test/intltest/nu...
File icu4c/source/test/intltest/numbertest_decimalquantity.cpp (right):

https://codereview.appspot.com/340990043/diff/1/icu4c/source/test/intltest/nu...
icu4c/source/test/intltest/numbertest_decimalquantity.cpp:298: void
DecimalQuantityTest::testToDouble() {
On 2018/04/03 18:32:52, andy.heninger wrote:
> Does the new double implementation work with the old IBM mainframe double
> format?

The answer depends on whether double_conversion supports it.

https://codereview.appspot.com/340990043/diff/1/icu4c/source/test/intltest/nu...
icu4c/source/test/intltest/numbertest_decimalquantity.cpp:304: { "-3.142E-271",
-3.142e-271 } };
On 2018/04/03 18:32:52, andy.heninger wrote:
> What is the significance of these specific numbers?

It's an example of a number that was failing to convert accurately using my
previous double-to-string implementation.

[andy.heninger, 2018-04-25 23:37:28]

LGTM. Trac ticket closed.