| Author |
Message |
|
| Guest |
 Posted: Sun May 30, 2010 12:44 am |
 |
|
|
Guest
|
This is a submission of Serge Aleynikov's ei patch that implements
binary encoding for floats:
git fetch git://github.com/vinoski/otp.git serge_new_float_ext
Note there are two commits on this branch: 3b3bae9 and a1ae64e. One
just fixes a typo in the ei_decode_ei_term documentation -- I kept
that commit separate since it's independent. The other commit is
Serge's original R12B-2 patch adjusted as needed for the current code
base and with the addition of fixing all areas across Erlang/OTP
affected by the change. The only exception is that I didn't change
erl_marshal.c because it's "legacy" so I'm not sure it has to handle
this new encoding or not.
If anyone wants to see the exact changes for each commit, the links to
the two commits are:
http://github.com/vinoski/otp/commit/3b3bae9397b405ce5e782d684ed88e0442eab65e
http://github.com/vinoski/otp/commit/a1ae64e5c30f89a2c6b9d1b83777a72235ffc003
--steve
________________________________________________________________
erlang-patches (at) erlang.org mailing list.
See http://www.erlang.org/faq.html
To unsubscribe; mailto:erlang-patches-unsubscribe@erlang.org
Post received from mailinglist |
|
|
| Back to top |
|
| Guest |
| Posted: Sun May 30, 2010 12:44 am |
|
|
|
Guest
|
This is a submission of Serge Aleynikov's ei patch that implements
binary encoding for floats:
git fetch git://github.com/vinoski/otp.git serge_new_float_ext
Note there are two commits on this branch: 3b3bae9 and a1ae64e. One
just fixes a typo in the ei_decode_ei_term documentation -- I kept
that commit separate since it's independent. The other commit is
Serge's original R12B-2 patch adjusted as needed for the current code
base and with the addition of fixing all areas across Erlang/OTP
affected by the change. The only exception is that I didn't change
erl_marshal.c because it's "legacy" so I'm not sure it has to handle
this new encoding or not.
If anyone wants to see the exact changes for each commit, the links to
the two commits are:
http://github.com/vinoski/otp/commit/3b3bae9397b405ce5e782d684ed88e0442eab65e
http://github.com/vinoski/otp/commit/a1ae64e5c30f89a2c6b9d1b83777a72235ffc003
--steve
________________________________________________________________
erlang-patches (at) erlang.org mailing list.
See http://www.erlang.org/faq.html
To unsubscribe; mailto:erlang-patches-unsubscribe@erlang.org
Post received from mailinglist |
|
|
| Back to top |
|
| Guest |
| Posted: Sun May 30, 2010 1:18 am |
|
|
|
Guest
|
Steve,
Thanks for taking time to rework it for proper submission!
Serge
On 5/29/2010 8:44 PM, Steve Vinoski wrote:
> This is a submission of Serge Aleynikov's ei patch that implements
> binary encoding for floats:
>
> git fetch git://github.com/vinoski/otp.git serge_new_float_ext
>
> Note there are two commits on this branch: 3b3bae9 and a1ae64e. One
> just fixes a typo in the ei_decode_ei_term documentation -- I kept
> that commit separate since it's independent. The other commit is
> Serge's original R12B-2 patch adjusted as needed for the current code
> base and with the addition of fixing all areas across Erlang/OTP
> affected by the change. The only exception is that I didn't change
> erl_marshal.c because it's "legacy" so I'm not sure it has to handle
> this new encoding or not.
>
> If anyone wants to see the exact changes for each commit, the links to
> the two commits are:
>
> http://github.com/vinoski/otp/commit/3b3bae9397b405ce5e782d684ed88e0442eab65e
> http://github.com/vinoski/otp/commit/a1ae64e5c30f89a2c6b9d1b83777a72235ffc003
>
> --steve
________________________________________________________________
erlang-patches (at) erlang.org mailing list.
See http://www.erlang.org/faq.html
To unsubscribe; mailto:erlang-patches-unsubscribe@erlang.org
Post received from mailinglist |
|
|
| Back to top |
|
| Guest |
| Posted: Sun May 30, 2010 1:18 am |
|
|
|
Guest
|
Steve,
Thanks for taking time to rework it for proper submission!
Serge
On 5/29/2010 8:44 PM, Steve Vinoski wrote:
> This is a submission of Serge Aleynikov's ei patch that implements
> binary encoding for floats:
>
> git fetch git://github.com/vinoski/otp.git serge_new_float_ext
>
> Note there are two commits on this branch: 3b3bae9 and a1ae64e. One
> just fixes a typo in the ei_decode_ei_term documentation -- I kept
> that commit separate since it's independent. The other commit is
> Serge's original R12B-2 patch adjusted as needed for the current code
> base and with the addition of fixing all areas across Erlang/OTP
> affected by the change. The only exception is that I didn't change
> erl_marshal.c because it's "legacy" so I'm not sure it has to handle
> this new encoding or not.
>
> If anyone wants to see the exact changes for each commit, the links to
> the two commits are:
>
> http://github.com/vinoski/otp/commit/3b3bae9397b405ce5e782d684ed88e0442eab65e
> http://github.com/vinoski/otp/commit/a1ae64e5c30f89a2c6b9d1b83777a72235ffc003
>
> --steve
________________________________________________________________
erlang-patches (at) erlang.org mailing list.
See http://www.erlang.org/faq.html
To unsubscribe; mailto:erlang-patches-unsubscribe@erlang.org
Post received from mailinglist |
|
|
| Back to top |
|
| Guest |
| Posted: Mon May 31, 2010 9:53 am |
|
|
|
Guest
|
On Sat, May 29, 2010 at 08:44:15PM -0400, Steve Vinoski wrote:
> This is a submission of Serge Aleynikov's ei patch that implements
> binary encoding for floats:
>
> git fetch git://github.com/vinoski/otp.git serge_new_float_ext
Thank you, will be included in 'pu' as sv/serge-new_float_ext.
>
> Note there are two commits on this branch: 3b3bae9 and a1ae64e. One
> just fixes a typo in the ei_decode_ei_term documentation -- I kept
> that commit separate since it's independent. The other commit is
> Serge's original R12B-2 patch adjusted as needed for the current code
> base and with the addition of fixing all areas across Erlang/OTP
> affected by the change. The only exception is that I didn't change
> erl_marshal.c because it's "legacy" so I'm not sure it has to handle
> this new encoding or not.
>
> If anyone wants to see the exact changes for each commit, the links to
> the two commits are:
>
> http://github.com/vinoski/otp/commit/3b3bae9397b405ce5e782d684ed88e0442eab65e
> http://github.com/vinoski/otp/commit/a1ae64e5c30f89a2c6b9d1b83777a72235ffc003
>
> --steve
>
> ________________________________________________________________
> erlang-patches (at) erlang.org mailing list.
> See http://www.erlang.org/faq.html
> To unsubscribe; mailto:erlang-patches-unsubscribe@erlang.org
--
/ Raimo Niskanen, Erlang/OTP, Ericsson AB
________________________________________________________________
erlang-patches (at) erlang.org mailing list.
See http://www.erlang.org/faq.html
To unsubscribe; mailto:erlang-patches-unsubscribe@erlang.org
Post received from mailinglist |
|
|
| Back to top |
|
| Guest |
| Posted: Mon May 31, 2010 9:54 am |
|
|
|
Guest
|
On Sat, May 29, 2010 at 08:44:15PM -0400, Steve Vinoski wrote:
> This is a submission of Serge Aleynikov's ei patch that implements
> binary encoding for floats:
>
> git fetch git://github.com/vinoski/otp.git serge_new_float_ext
Thank you, will be included in 'pu' as sv/serge-new_float_ext.
>
> Note there are two commits on this branch: 3b3bae9 and a1ae64e. One
> just fixes a typo in the ei_decode_ei_term documentation -- I kept
> that commit separate since it's independent. The other commit is
> Serge's original R12B-2 patch adjusted as needed for the current code
> base and with the addition of fixing all areas across Erlang/OTP
> affected by the change. The only exception is that I didn't change
> erl_marshal.c because it's "legacy" so I'm not sure it has to handle
> this new encoding or not.
>
> If anyone wants to see the exact changes for each commit, the links to
> the two commits are:
>
> http://github.com/vinoski/otp/commit/3b3bae9397b405ce5e782d684ed88e0442eab65e
> http://github.com/vinoski/otp/commit/a1ae64e5c30f89a2c6b9d1b83777a72235ffc003
>
> --steve
>
> ________________________________________________________________
> erlang-patches (at) erlang.org mailing list.
> See http://www.erlang.org/faq.html
> To unsubscribe; mailto:erlang-patches-unsubscribe@erlang.org
--
/ Raimo Niskanen, Erlang/OTP, Ericsson AB
________________________________________________________________
erlang-patches (at) erlang.org mailing list.
See http://www.erlang.org/faq.html
To unsubscribe; mailto:erlang-patches-unsubscribe@erlang.org
Post received from mailinglist |
|
|
| Back to top |
|
| Guest |
| Posted: Mon May 31, 2010 10:10 am |
|
|
|
Guest
|
This looks good but I have some concerns:
- Aesthetics, coding convention: please refrain from C++ commenting.
- Distribution flags, the DFLAG_NEW_FLOATS option should be enabled in
lib/erl_interface/src/connect/ei_connect.c: static int
send_name_or_challenge.
See lib/erl_interface/src/connect/ei_connect_int.h and
lib/kernel/src/dist.hrl for some details.
- I think that "legacy" should include this as well. It should,
unless someone can think of a reason not to include it.
Anyway, with DFLAGS_NEW_FLOATS flag enabled the code would be tested as
well which is a necessity in order for it to be included in the otp relase.
Regards,
Bj |
|
|
| Back to top |
|
| Guest |
| Posted: Mon May 31, 2010 10:11 am |
|
|
|
Guest
|
This looks good but I have some concerns:
- Aesthetics, coding convention: please refrain from C++ commenting.
- Distribution flags, the DFLAG_NEW_FLOATS option should be enabled in
lib/erl_interface/src/connect/ei_connect.c: static int
send_name_or_challenge.
See lib/erl_interface/src/connect/ei_connect_int.h and
lib/kernel/src/dist.hrl for some details.
- I think that "legacy" should include this as well. It should,
unless someone can think of a reason not to include it.
Anyway, with DFLAGS_NEW_FLOATS flag enabled the code would be tested as
well which is a necessity in order for it to be included in the otp relase.
Regards,
Bj |
|
|
| Back to top |
|
| Guest |
| Posted: Mon May 31, 2010 2:42 pm |
|
|
|
Guest
|
|
| Back to top |
|
| Guest |
| Posted: Mon May 31, 2010 2:42 pm |
|
|
|
Guest
|
|
| Back to top |
|
| Guest |
| Posted: Mon May 31, 2010 3:33 pm |
|
|
|
Guest
|
On 2010-05-31 16:42, Steve Vinoski wrote:
> 2010/5/31 Bj |
|
|
| Back to top |
|
| Guest |
| Posted: Mon May 31, 2010 3:33 pm |
|
|
|
Guest
|
On 2010-05-31 16:42, Steve Vinoski wrote:
> 2010/5/31 Bj |
|
|
| Back to top |
|
| Guest |
| Posted: Tue Jun 01, 2010 7:07 am |
|
|
|
Guest
|
I've pushed an amended commit to github that I believe addresses all
your suggestions:
git fetch git://github.com/vinoski/otp.git serge_new_float_ext
Thanks to your feedback, I found a number of other places that needed
attention as part of this patch. If you could take another careful
look at the new changes, that would be good.
More comments below.
2010/5/31 Bj |
|
|
| Back to top |
|
| Guest |
| Posted: Tue Jun 01, 2010 7:07 am |
|
|
|
Guest
|
I've pushed an amended commit to github that I believe addresses all
your suggestions:
git fetch git://github.com/vinoski/otp.git serge_new_float_ext
Thanks to your feedback, I found a number of other places that needed
attention as part of this patch. If you could take another careful
look at the new changes, that would be good.
More comments below.
2010/5/31 Bj |
|
|
| Back to top |
|
| Guest |
| Posted: Tue Jun 01, 2010 1:03 pm |
|
|
|
Guest
|
On 2010-06-01 09:06, Steve Vinoski wrote:
> I've pushed an amended commit to github that I believe addresses all
> your suggestions:
>
> git fetch git://github.com/vinoski/otp.git serge_new_float_ext
>
> Thanks to your feedback, I found a number of other places that needed
> attention as part of this patch. If you could take another careful
> look at the new changes, that would be good.
Oops. I forgot something. (And yet again erl_interface is faced with a
problem). ei_get_type exposes the internal representation (external
format) of types. This is a problem. Earlier implementation had its own
types ... which seemingly was a bad idea ...
Angels are crying for suggesting this but, what do you think of letting
get_type return ERL_FLOAT_EXT for both ERL_FLOAT_EXT and NEW_FLOAT_EXT?
My aim is to remove the necessity for changing original source code
dependent on erl_interface, if that is at all possible.
What do you think?
Hmm, actually this is somewhat more serious then I first thought. I need
to talk it over with my team.
> I reinstated these tests and modified them to match the style of other
> tests in the same suite. I also found that match_float could
> incorrectly fail when dealing with the float value 0.0 so I augmented
> it with another function clause that allows for simple equality.
Great!
// Bj |
|
|
| Back to top |
|
|
|
Digg It
Del.icio.us
Reddit
Facebook
Stumble Upon
Technorati
