[erlang-patches] [patch] new float_to_list/2

Sat Apr 21 07:19:04 CEST 2012

Hi Henrik,

I updated the patch to be based on maint and fixed the comment per your 
instructions:

git fetch https://github.com/saleyn/otp/tree/float_to_list_2

https://github.com/saleyn/otp/compare/maint...float_to_list_2
https://github.com/saleyn/otp/compare/maint...float_to_list_2.patch

Regards,

Serge

On 4/20/2012 5:17 AM, Henrik Nord wrote:
> Hi
>
> Could you fix your commit messages to conform with the guidelines found
> here:
> https://github.com/erlang/otp/wiki/Writing-good-commit-messages
>
>
> And I would also request that you rebase your branch upon the 'maint'
> branch
>
> Thank you for the contribution!
>
> On 04/18/2012 05:43 PM, Serge Aleynikov wrote:
>> Hi Sverker,
>>
>> The following branch/patch contains the changes you requested. The
>> changes were tested on Linux (I currently don't have access to other
>> platforms).
>>
>> #1 Using erts_snprintf
>> #2 Moved redundant code to
>> erts/emulator/sys/common/erl_sys_common_misc.c
>>
>> git fetch git://github.com/saleyn/otp.git float_to_list_2
>>
>> https://github.com/saleyn/otp/compare/float_to_list_2
>> https://github.com/saleyn/otp/compare/float_to_list_2.patch
>>
>> Also fixed application the new "compact" version on large floats (with
>> exponent exceeding 1 << 53 digits) for which erts_snprintf is used.
>>
>> Please let me know if there are any more issues on any other platform.
>>
>> Regards,
>>
>> Serge
>>
>> On 3/13/2012 1:15 PM, Sverker Eriksson wrote:
>>>
>>>
>>> #1. The patch does not compile on Windows:
>>>
>>> error LNK2019: unresolved external symbol _snprintf referenced in
>>> function _sys_double_to_chars_fast
>>>
>>> Use erts_snprintf instead. Same as snprintf with extra %T feature for
>>> printing Erlang terms.
>>>
>>>
>>> #2. We don't like identical source copies of sys_double_to_chars_fast().
>>> There is a directory sys/common for shared code.
>>>
>>>
>>> /Sverker, Erlang/OTP
>>>
>>>
>>> Gustav Simonsson wrote:
>>>> Hi Serge,
>>>>
>>>> We'll get back to you about this patch after the ERTS team have
>>>> reviewed it.
>>>> Thank you for the contribution!
>>>>
>>>> Regards,
>>>> Gustav Simonsson
>>>> Erlang/OTP team
>>>>
>>>> On 2012-03-08 19:03, Serge Aleynikov wrote:
>>>>> Dear Sverker,
>>>>>
>>>>> Sorry for a belated response. The following commit addresses three
>>>>> issues you indicated. Attached is also a small performance test
>>>>> program which illustrates that the new float_to_list/2 is about 6x
>>>>> faster than float_to_list/1:
>>>>>
>>>>> https://github.com/saleyn/otp/commit/d7a108f28fd8cd519852feb0233920af511b5eba
>>>>>
>>>>>
>>>>>
>>>>> git fetch git://github.com/saleyn/otp.git float_to_list_2
>>>>>
>>>>> https://github.com/saleyn/otp/compare/float_to_list_2
>>>>> https://github.com/saleyn/otp/compare/float_to_list_2.patch
>>>>>
>>>>>
>>>>> Executing the attachment test function:
>>>>> 1> test:test(1.0).
>>>>> float_to_list(1.000000, []) = {0.149531,"1.000000"}
>>>>> float_to_list(1.000000, [{decimals, 4}]) = {0.109001,"1.0000"}
>>>>> float_to_list(1.000000) = {0.629831,"1.00000000000000000000e+00"}
>>>>> ok
>>>>>
>>>>> Regards,
>>>>>
>>>>> Serge
>>>>>
>>>>> P.S. I don't have access to solaris and freebsd at the moment, but
>>>>> the code works on 32/64-bit linux, and is identical for those two
>>>>> platforms.
>>>>>
>>>>> On 3/28/2011 11:39 AM, Sverker Eriksson wrote:
>>>>>> Thanks for your patch. As you may have seen it did not make it into
>>>>>> R14B02.
>>>>>>
>>>>>> Comments:
>>>>>>
>>>>>> 1. Test fail on some platforms (32-bit solaris, freebsd and 64-bit
>>>>>> linux)
>>>>>>
>>>>>> float_to_list(1.0,[compact])
>>>>>>
>>>>>> returns "1." instead of "1.0"
>>>>>>
>>>>>>
>>>>>> 2. Would like the interface to be extendable to support printf's
>>>>>> %e and
>>>>>> %g formats in future. Maybe just rename 'precision' to 'decimals'.
>>>>>>
>>>>>> 3. Why is default 4 decimals when printf and io:format has 6 as
>>>>>> default.
>>>>>>
>>>>>>
>>>>>> /Sverker, Erlang/OTP
>>>>>>
>>>>>>
>>>>>>
>>>>>>
>>>>>> Serge Aleynikov wrote:
>>>>>>> This implementation has been submitted as the pull request:
>>>>>>>
>>>>>>> https://github.com/erlang/otp/pull/9
>>>>>>>
>>>>>>> This version is also optimized to run 5-10x faster than
>>>>>>> float_to_list/1 for floats under 2^52.
>>>>>>>
>>>>>>> Regards,
>>>>>>>
>>>>>>> Serge
>>>>>>>
>>>>>>> On 1/14/2011 10:33 AM, Gleb Peregud wrote:
>>>>>>>> Here's my patch:
>>>>>>>>
>>>>>>>> http://www.erlang.org/cgi-bin/ezmlm-cgi/4/43529
>>>>>>>>
>>>>>>>> It uses double sprintf invocations. Your solution is clearly
>>>>>>>> better (I
>>>>>>>> didn't knew it is possible to specify precision in args).
>>>>>>>>
>>>>>>>> Your benchmark shows that difference is negligible. Between
>>>>>>>> integer as
>>>>>>>> a second parameter or a proplist (~0.4%). Though it might be a bit
>>>>>>>> more "stable" in terms of running time, but still probably not
>>>>>>>> worth
>>>>>>>> considering it for performance reasons. It still may be useful in
>>>>>>>> terms of simpler API, but I have no strong opinion on it.
>>>>>>>>
>>>>>>>> So generally +1 on including Serge's patch into Erlang. Serge, can
>>>>>>>> you
>>>>>>>> put it into GitHub pull request? As described here:
>>>>>>>>
>>>>>>>> https://github.com/erlang/otp/wiki/submitting-patches
>>>>>>>>
>>>>>>>> Best,
>>>>>>>> Gleb
>>>>>>>>
>>>>>>>> On Wed, Jan 12, 2011 at 18:13, Serge Aleynikov<serge@REDACTED>
>>>>>>>> wrote:
>>>>>>>>> I did a micro-benchmark on a slightly modified version of the BIF
>>>>>>>>> that
>>>>>>>>> accepts an integer as its options. The results shown below
>>>>>>>>> display a
>>>>>>>>> very
>>>>>>>>> insignificant difference between a call with no options and a call
>>>>>>>>> with an
>>>>>>>>> integer precision passed as the second argument:
>>>>>>>>>
>>>>>>>>> 1> test:test().
>>>>>>>>> float_to_list(123.4, []) = {0.619512,"123.4000"}
>>>>>>>>> float_to_list(123.4, [{precision, 4}]) = {0.624895,"123.4000"}
>>>>>>>>> float_to_list(123.4, 4) = {0.622896,"123.4000"}
>>>>>>>>>
>>>>>>>>> The majority of time is actually spent in the printf(3) function,
>>>>>>>>> which
>>>>>>>>> takes longer to execute when given the "%.*f" argument compared to
>>>>>>>>> "%g" as
>>>>>>>>> in float_to_list/1 case.
>>>>>>>>>
>>>>>>>>> Did your patch rely on printf?
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> On 1/12/2011 9:23 AM, Gleb Peregud wrote:
>>>>>>>>>>
>>>>>>>>>> Some time ago I've submitted similar but simpler patch. It was a
>>>>>>>>>> float_to_list/2 with a second parameter being an integer
>>>>>>>>>> specifying
>>>>>>>>>> precision. For me it was important to generate A LOT of floats as
>>>>>>>>>> strings as fast as possible with specified precision. Serge's
>>>>>>>>>> version
>>>>>>>>>> has an overhead of inspecting proplist of the second parameter.
>>>>>>>>>> So I
>>>>>>>>>> was wondering about introducing two versions of this function:
>>>>>>>>>> with a
>>>>>>>>>> proplist as a second parameter and with a number as a second
>>>>>>>>>> parameter. Alternatively proplist version can be factored out
>>>>>>>>>> into
>>>>>>>>>> float_to_list_opts/2.
>>>>>>>>>>
>>>>>>>>>> Just my 0.2 cents
>>>>>>>>>>
>>>>>>>>>> On Wed, Jan 12, 2011 at 15:10, Serge
>>>>>>>>>> Aleynikov<serge@REDACTED>
>>>>>>>>>> wrote:
>>>>>>>>>>>
>>>>>>>>>>> The reason I called it precision was to be consistent with the
>>>>>>>>>>> naming
>>>>>>>>>>> convention of the printf function. Below is the extract from
>>>>>>>>>>> "man 3
>>>>>>>>>>> printf", which refers to the digits after the decimal point as
>>>>>>>>>>> "precision":
>>>>>>>>>>>
>>>>>>>>>>> f, F The double argument is rounded and converted to decimal
>>>>>>>>>>> notation in the style [-]ddd.ddd, where the number of
>>>>>>>>>>> digits after the decimal-point character is equal to the
>>>>>>>>>>> precision specification. If the precision is missing,
>>>>>>>>>>> it is taken as 6; if the precision is explicitly zero,
>>>>>>>>>>> no decimal-point character appears. If a decimal point
>>>>>>>>>>> appears, at least one digit appears before it.
>>>>>>>>>>>
>>>>>>>>>>> I don't have a very strong preference for calling it
>>>>>>>>>>> precision or
>>>>>>>>>>> scale,
>>>>>>>>>>> but
>>>>>>>>>>> I do have a strong preference for including this patch in the
>>>>>>>>>>> distribution,
>>>>>>>>>>> because the default behavior of float_to_list/1 hard-coded in
>>>>>>>>>>> C is
>>>>>>>>>>> deficient.
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> On 1/12/2011 4:58 AM, nox wrote:
>>>>>>>>>>>>
>>>>>>>>>>>> Il should be called "scale", shouldn't it?
>>>>>>>>>>>>
>>>>>>>>>>>> Le 12 janv. 2011 à 10:26, Pierpaolo
>>>>>>>>>>>> Bernardi<olopierpa@REDACTED> a
>>>>>>>>>>>> écrit
>>>>>>>>>>>> :
>>>>>>>>>>>>
>>>>>>>>>>>>> On Wed, Jan 12, 2011 at 06:44, Serge
>>>>>>>>>>>>> Aleynikov<serge@REDACTED>
>>>>>>>>>>>>> wrote:
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> Attached please find a patch that adds a new float_to_list/2
>>>>>>>>>>>>>> BIF. The
>>>>>>>>>>>>>> patch
>>>>>>>>>>>>>> was created off of the master branch of
>>>>>>>>>>>>>> https://github.com/erlang/otp.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> This BIF solves a problem of float_to_list/1 that doesn't
>>>>>>>>>>>>>> allow
>>>>>>>>>>>>>> specifying
>>>>>>>>>>>>>> the number of digits after the decimal point when formatting
>>>>>>>>>>>>>> floats.
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> float_to_list(Float, Options) -> string()
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> Float = float()
>>>>>>>>>>>>>> Options = [Option]
>>>>>>>>>>>>>> Option = {precision, Precision::integer()} | compact
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> Text representation of a float formatted using given options
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> Returns a string which corresponds to the text
>>>>>>>>>>>>>> representation of Float using fixed decimal point formatting.
>>>>>>>>>>>>>> When precision option is specified
>>>>>>>>>>>>>> the returned value will contain at most Precision number of
>>>>>>>>>>>>>> digits past the decimal point. When compact option is
>>>>>>>>>>>>>> provided
>>>>>>>>>>>>>> the trailing zeros at the end of the list are truncated.
>>>>>>>>>>>>>
>>>>>>>>>>>>> I think the option is misnamed.
>>>>>>>>>>>>>
>>>>>>>>>>>>> In the usual terminology, 'precision' is the total number of
>>>>>>>>>>>>> significative digits, not only the ones past the decimal
>>>>>>>>>>>>> point.
>>>>>>>>>>>>>
>>>>>>>>>>>>> Cheers
>>>>>>>>>>>>> P.
>>>>>>>>>>>>>
>>>>>>>>>>>>> ________________________________________________________________
>>>>>>>>>>>>>
>>>>>>>>>>>>> erlang-questions (at) erlang.org mailing list.
>>>>>>>>>>>>> See http://www.erlang.org/faq.html
>>>>>>>>>>>>> To unsubscribe; mailto:erlang-questions-unsubscribe@REDACTED
>>>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> ________________________________________________________________
>>>>>>>>>>> erlang-questions (at) erlang.org mailing list.
>>>>>>>>>>> See http://www.erlang.org/faq.html
>>>>>>>>>>> To unsubscribe; mailto:erlang-questions-unsubscribe@REDACTED
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>
>>>>>>>
>>>>>>> ________________________________________________________________
>>>>>>> erlang-patches (at) erlang.org mailing list.
>>>>>>> See http://www.erlang.org/faq.html
>>>>>>> To unsubscribe; mailto:erlang-patches-unsubscribe@REDACTED
>>>>>>>
>>>>>>
>>>>>>
>>>>>
>>>>> _______________________________________________
>>>>> erlang-patches mailing list
>>>>> erlang-patches@REDACTED
>>>>> http://erlang.org/mailman/listinfo/erlang-patches
>>>>
>>>>
>>>
>>>
>> _______________________________________________
>> erlang-patches mailing list
>> erlang-patches@REDACTED
>> http://erlang.org/mailman/listinfo/erlang-patches
>