spec: use int64, allow counter-flow messages #71
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
It makes sense to use float64 where the measurement unit is a floating, but using it when we're sending integer quantities is actually a bit surprising. While it's true that `float64` is native to the i386 platform and `int64` it's not, I don't even know whether `int64` is slower than `float64` and, even if that was the case, it would probably be weird anyway to use `float64` rather than `int64`. Another reason why I was using `float64` is that I didn't know how to extact `int64`s from C code. But C code is gone in the previous commit. Conclusion: let's use `int64` instead. Now, why `int64` and not `uint64`? The different is subtle. We know that in many cases golang is using signed integers also for sizes, so that is not so surprising in golang. In Java there is no unsigned, so in theory I could say I am doing this for the sake of good old Java. The reality is that JSON cannot really serialize numbers larger than 2^53 as integers, therefore the distinction betwwn `int64` and `uint64` here is academic. Thus, I have picked the most natural feat in golang. (Should I check for overflow? In theory. In practice I think we are good for quite some years without checking for overflow of `int64` variables. Also, in the server side there are checks for overflow of `int64`, but not of `int53`. In the client side I think we should not care, since that is a minimal client.)
Pull Request Test Coverage Report for Build 446
💛 - Coveralls |
pboothe
reviewed
Mar 4, 2019
It is okay. I'll clarify that, thanks! |
Also, we don't have UDP overhead that we know of.
As mentioned in the spec, this is quite an academic concern for the moment and most likely in the future. However, clarify we have thought about this issue, we currently consider it relevant for pedantic implementations only, and recommend pedantic code to make sure the int53 boundary is correctly respected. We may change our mind in the future. (The most likely course of action is that we'll specify what a pedantic implementation is supposed to do in such case. For now it doesn't matter.)
Do not require these messages to be sent. Yet, require a party to be prepared to receive these messages. The reason why I'm doing this now is to take advantage of the recent bump in the specification version.
bassosimone
changed the title
pboothe
approved these changes
Mar 5, 2019
There was a problem hiding this comment.
Reviewable status:
complete! 1 of 1 approvals obtained (waiting on @pboothe)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
It makes sense to use float64 where the measurement unit is
a floating, but using it when we're sending integer quantities
is actually a bit surprising.
While it's true that
float64is native to the i386 platformand
int64it's not, I don't even know whetherint64is slowerthan
float64and, even if that was the case, it would probablybe weird anyway to use
float64rather thanint64.Another reason why I was using
float64is that I didn't knowhow to extact
int64s from C code. But C code is gone in theprevious commit. Conclusion: let's use
int64instead.Now, why
int64and notuint64? The different is subtle. Weknow that in many cases golang is using signed integers also
for sizes, so that is not so surprising in golang. In Java there
is no unsigned, so in theory I could say I am doing this for
the sake of good old Java. The reality is that JSON cannot really
serialize numbers larger than 2^53 as integers, therefore the
distinction betwwn
int64anduint64here is academic. Thus,I have picked the most natural feat in golang.
(Should I check for overflow? In theory. In practice I think we
are good for quite some years without checking for overflow of
int64variables. Also, in the server side there are checks foroverflow of
int64, but not ofint53. In the client side Ithink we should not care, since that is a minimal client.)
In addition to the above, there is also a17f14f that reallows
counter-flow messages. Will make upload better.
This change is