search

alecthomas / go_serialization_benchmarks

Benchmarks of Go serialization methods
https://alecthomas.github.io/go_serialization_benchmarks/
1.56k stars 158 forks source link

Bebop input data #126

Closed ymz-ncnk closed 9 months ago

ymz-ncnk commented 9 months ago

It turns out that Bebop uses different from others input data, in generateBebop200sc(): Birthday: time.Now().Round(100 * time.Nanosecond),

I believe that the input should be the same for everyone so that everyone has the same conditions @200sc.

200sc commented 9 months ago

This is noted in the readme: 

Bebop (both libraries, by nature of the format) natively supports times rounded to 100ns ticks, and this is what is currently benchmarked (a unix nano timestamp is another valid approach).

If this were to be unrounded validation tests would fail, because the data format does not support it. As also noted, you could just use a unix nano int64, like FlatBuffers, ProtoBuffers, Cap'N'Proto and ikeapack; were we to make this change we would expect a marginal bump in encoding and decoding speed for the bebop libraries as they perform extra work to parse and un-round the timestamp type, so IMO it is fairer to use the native, less efficient format.

ymz-ncnk commented 9 months ago

I believe that benchmarks should be run with the same input data. Especially when there is such a possibility.

200sc commented 9 months ago

As mentioned, there isn't, and this restriction would forbid very popular encoders like Cap'N'Proto, unless you're suggesting the int64 timestamp variant, which, as mentioned again, would be faster and not reflective of the bebop API.

ymz-ncnk commented 9 months ago

It is not for me to suggest which method to use. But how do you imagine benchmarks in which different participants use different inputs? Can we do that in sports competitions too?

200sc commented 9 months ago

Maybe the problem here is viewing this as a competition, and not a reflection of different APIs and capabilities. If you feel the note in the readme is insufficient, I'm sure a PR augmenting it or making your preferred change would be reviewed.

alecthomas commented 9 months ago

It's fine to use a timestamp, there are a few serialisation approaches that do the same because they don't natively support time.Time. As long as it's noted in the README so that people are informed, I'm fine with it.