Use jute generated structs

[nemith]

I spent some time writing https://github.com/go-zookeeper/jute which is nearly complete which can auto generate structures and how to serialize/deserialize them to the wire similar to Java which will illuminate the reflection and bring a bit more protection as well.

[pmazzini]

This is interesting!

[jeffbean]

I'm leaning more now to not build and maintain something that supports what at this point is a dead IDL. I think the value of maintaining our structs is lower since ZK itself does not break any backwards compatibility.

Adding this is really going to be low ROI and falls into the boat of keeping simple may be best for the repo.

In place of that I think organizing, and better documenting the models we do maintain, as well as making some level of effort to follow and do some proactive additions when ZK major revs would be sufficient.

[nemith]

You are correct at the time this task was created i think it was less clear. It has been some years.

That being said most of the work as been done. Using jute is a matter of simple implementation. We didn't do it before as an intern at Facebook implemented a new zk library that used jute but that never went forward. It still exists here https://github.com/facebookarchive/zk. Facebook/Meta no longer use this API for ZK servers (and i think they moved away from zookeeper all together).

The win isn't the IDL. The win is NOT using reflection to send messages . You can manually implement the message reading/writing without reflection just as well but will take a lot more work.

So it's more than just maintaining struct imho.

[nemith]

Remember that jute define not only the structs but also the encoding.

[nemith]

More evidence:

with existing reflection encoding

package zk

import "testing"

func BenchmarkPacketEncode(b *testing.B) {
	var buf []byte

	packet := connectRequest{
		ProtocolVersion: 0,
		LastZxidSeen:    0,
		TimeOut:         2000,
		SessionID:       0,
		Passwd:          []byte("foo"),
	}

	for i := 0; i < b.N; i++ {
		encodePacket(buf, &packet)
	}
}

with jute (no reflection) encoding

$ cat bench_test.go
package proto

import (
        "bytes"
        "testing"

        "github.com/go-zookeeper/jute/lib/go/jute"
)

func BenchmarkEncode(b *testing.B) {
        var buf bytes.Buffer
        enc := jute.NewBinaryEncoder(&buf)

        packet := ConnectRequest{
                ProtocolVersion: 0,
                LastZxidSeen:    0,
                TimeOut:         2000,
                SessionId:       0,
                Passwd:          []byte("foo"),
        }

        for i := 0; i < b.N; i++ {
                enc.Encode(&packet)
        }
}

Results

$ go test . -bench=BenchmarkPacketEncode -run=NONE
goos: darwin
goarch: arm64
pkg: github.com/go-zookeeper/zk
BenchmarkPacketEncode-10         4097984               286.3 ns/op
PASS
ok      github.com/go-zookeeper/zk      1.629s

$ go test . -bench=BenchmarkEncode -run=NONE
goos: darwin
goarch: arm64
pkg: github.com/go-zookeeper/zk/internal/proto
BenchmarkEncode-10      30879982                38.96 ns/op
PASS
ok      github.com/go-zookeeper/zk/internal/proto       1.631s

A 7.3x improvement may be worth it. Especially that all the code gen stuff is already done. It's literally just repumbing with new messages. The reason it hasn't been done yet is that messages are exported and it's a breaking change.

This also gets better as the old version writes (and allocates) buffers on each write where the new one can write directly to a buffered socket.

The IDL sucks, but so does reflection.

[nemith]

For compat you can look into making the generated code more compatible or just writing translations between existing structs (my plan before i left FB) . Ideally structs are not exported and them being exported is also a mistake. Both can be fixed in a proper v2 version (which there is a lot of api that needs cleaning up)

[jeffbean]

good points. I'm thinking how to sustain this lib and adding this adds a bit of complexity. That being said since the jute stuff most likely is also frozen, once the work to do this is done its most likely not going to change from underneath us.

v2 I'm not sure what it really looks like yet ... I'm kind of hoping to get a bunch of the v1 bugs and PRs cleaned up and cut a v2 dev or something where we can start to break things in serious ways.

I'm just warming back up to be active once again, jute was the thing I am most unfamiliar and uncomfortable supporting. If the code gen is in a good place then yeah, its a big win