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

proto: specialize map<string, string> for performance #781

Open
jmillikin-stripe opened this issue Jan 8, 2019 · 3 comments
Open

proto: specialize map<string, string> for performance #781

jmillikin-stripe opened this issue Jan 8, 2019 · 3 comments

Comments

@jmillikin-stripe
Copy link

Background: #624

Serialization of map fields currently hits a slow, reflection-based path. This is understandable for the general case (nested messages are hard), but there are some special cases where maps get used a lot and a fast path would be very valuable.

My specific use case is a map<string, string> used to represent arbitrary human-meaningful metadata in a distributed monitoring system (similar to Prometheus). Benchmarking shows that our tooling is spending a material amount of CPU time in the closures returned from proto.makeMapMarshaler.

@dsnet dsnet changed the title Please specialize map<string, string> for performance proto: specialize map<string, string> for performance Jan 8, 2019
@bartle-stripe
Copy link

fwiw, I wrote a quick benchmark for comparing against gogo faster:

func BenchmarkMarshalSSFSpan(b *testing.B) {
    s := &ssf.SSFSpan{
        Tags: make(map[string]string),
    }
    for i := 0; i < 20; i++ {
        s.Tags[strconv.Itoa(i)] = strconv.Itoa(i * 1000)
    }

    b.ResetTimer()
    b.ReportAllocs()
    for i := 0; i < b.N; i++ {
        _, err := proto.Marshal(s)
        if err != nil {
            panic(err)
        }
    }
}

SSFSpan is from https://github.com/stripe/veneur/blob/master/ssf/sample.proto.

BenchmarkMarshalSSFSpan-4         100000         15003 ns/op        3952 B/op        165 allocs/op
BenchmarkMarshalSSFSpan-4         500000          2413 ns/op         240 B/op          1 allocs/op

Top is protoc-gen-go, bottom is protoc-gen-gogofaster.

@prannayk
Copy link
Contributor

If nobody is working on this I would like to get assigned to this.

@dsnet
Copy link
Member

dsnet commented Nov 17, 2020

Making a note that this should probably also handle map[string]*Message.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants