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

work-in-progress for fixing pulsar 1.0 blockers #109

Draft
wants to merge 11 commits into
base: main
Choose a base branch
from

Conversation

elias-orijtech
Copy link

Work in progress for pulsar 1.0, so far fixing #50 and #37. Builds upon #108 to avoid merge conflicts while it's in review.

@aaronc @tac0turtle in light of #2 (comment), is there a point in continuing this work?

CC @odeke-em

The ProtoMethods is called for every unmarshal, which in turn allocates
an protoiface.Methods. The Methods object does not refer to a partucular
message object, so the allocation can be saved by one-time initialization
at startup.

Found by the the alecthomas/go_serialization_benchmarks suite:

    go test -bench='Pulsar' -memprofile mem.out  -cpu 6  ./

Before:
goos: darwin
goarch: arm64
pkg: github.com/alecthomas/go_serialization_benchmarks
Benchmark_Pulsar_Marshal-6     	 7480744	       154.1 ns/op	        51.57 B/serial	      96 B/op	       2 allocs/op
Benchmark_Pulsar_Unmarshal-6   	 5057922	       236.9 ns/op	        51.60 B/serial	     256 B/op	       7 allocs/op

After:
Benchmark_Pulsar_Marshal-6     	 7251019	       153.9 ns/op	        51.57 B/serial	      96 B/op	       2 allocs/op
Benchmark_Pulsar_Unmarshal-6   	 6409070	       187.2 ns/op	        51.63 B/serial	     160 B/op	       5 allocs/op
CheckInitialized checks for missing required fields, which is not
relevant for a proto3 implementation such as Pulsar. However, without it
protobuf will fall back to a slow fallback that allocates memory per
serialization.

Before:

Benchmark_Pulsar_Marshal-6       7251019               153.9 ns/op              51.57 B/serial        96 B/op          2 allocs/op
Benchmark_Pulsar_Unmarshal-6     6409070               187.2 ns/op              51.63 B/serial       160 B/op          5 allocs/op

After:

Benchmark_Pulsar_Marshal-6     	12480471	        91.47 ns/op	        51.61 B/serial	     128 B/op	       2 allocs/op
Benchmark_Pulsar_Unmarshal-6   	10919157	       108.6 ns/op	        51.58 B/serial	     128 B/op	       3 allocs/op
Solve the clashes by introducing a level of indirection, replacing

type fastReflection_A A

with

type fastReflection_A struct {
 x *A
}

Updates cosmos#50
Now that fast reflection has struct wrappers, field/method name clashes
are no longer possible.

Fixes cosmos#50
@elias-orijtech elias-orijtech requested a review from a team as a code owner March 24, 2023 23:47
@elias-orijtech elias-orijtech marked this pull request as draft March 24, 2023 23:47
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.

1 participant