Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make wiretypes into julia types and specialize methods on these types #188

Merged
merged 1 commit into from
Dec 1, 2021

Conversation

neveritt
Copy link
Contributor

This PR removes the need for the WIRETYPES lookup table by specializing the lookup based on Julia types instead.
The lookup table is replace by wire_type, _read_value and _write_value functions.
This has a significant performance improvement for writing, but does not seem affect reading.

For example:

julia> using ProtoBuf, BenchmarkTools
julia> using ProtoBuf.google.protobuf

julia> dv = DoubleValue(value=64.0)
julia> dvmeta = ProtoBuf.meta(typeof(dv))
julia> @benchmark writeproto(iob, dv, dvmeta) setup = (iob = PipeBuffer();)
BenchmarkTools.Trial: 10000 samples with 308 evaluations.
 Range (min  max):  279.825 ns   14.253 μs  ┊ GC (min  max): 0.00%  97.90%
 Time  (median):     283.951 ns               ┊ GC (median):    0.00%
 Time  (mean ± σ):   297.450 ns ± 188.815 ns  ┊ GC (mean ± σ):  0.87% ±  1.37%

compared to

@benchmark writeproto(iob, dv, dvmeta) setup = (iob = PipeBuffer();)
BenchmarkTools.Trial: 10000 samples with 193 evaluations.
 Range (min  max):  502.933 ns    8.296 μs  ┊ GC (min  max): 0.00%  93.58%
 Time  (median):     512.648 ns               ┊ GC (median):    0.00%
 Time  (mean ± σ):   533.061 ns ± 106.134 ns  ┊ GC (mean ± σ):  0.15% ±  0.94%

in the previous version.

In order to achieve the specialization for the types that has several serializations, e.g. (int64, sint64, sfixed64)
two types SignedNumber and FixedSizeNumber are introduced to carry extra information about serialization.

I had to rearrange some methods to make the dispatch work properly, which may make the PR a bit hard to read.

@tanmaykm
Copy link
Member

tanmaykm commented Dec 1, 2021

LGTM. Thanks!

@tanmaykm tanmaykm merged commit 4f1f412 into JuliaIO:master Dec 1, 2021
@blegat blegat mentioned this pull request Dec 5, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants