tikv / pd

Placement driver for TiKV
Apache License 2.0
1.03k stars 714 forks source link

Not need `json.Unmarshal` when `Clone()`, which is time-consuming #8298

Closed okJiang closed 2 weeks ago

okJiang commented 2 weeks ago

Enhancement Task

https://github.com/tikv/pd/blob/c1d422ec4924799a5ff7da6606644959e339f1c0/pkg/schedule/placement/rule.go#L92-L93

func BenchmarkCloneWithJSON(b *testing.B) {
    r := &Rule{
        GroupID:  "g1",
        ID:       "r1",
        StartKey: []byte("start"),
        EndKey:   []byte("end"),
    }
    b.ResetTimer()

    for i := 0; i < b.N; i++ {
        var clone Rule
        json.Unmarshal([]byte(r.String()), &clone)
        clone.StartKey = append(r.StartKey[:0:0], r.StartKey...)
        clone.EndKey = append(r.EndKey[:0:0], r.EndKey...)
    }
}

func BenchmarkCloneWithAssignment(b *testing.B) {
    r := &Rule{
        GroupID:  "g1",
        ID:       "r1",
        StartKey: []byte("start"),
        EndKey:   []byte("end"),
    }
    b.ResetTimer()

    for i := 0; i < b.N; i++ {
        clone := *r
        clone.StartKey = append(clone.StartKey[:0:0], r.StartKey...)
        clone.EndKey = append(clone.EndKey[:0:0], r.EndKey...)
    }
}
goos: darwin
goarch: arm64
pkg: github.com/tikv/pd/pkg/schedule/placement
BenchmarkCloneWithJSON-8              735308          1961 ns/op         784 B/op         12 allocs/op
BenchmarkCloneWithAssignment-8      30597870            34.23 ns/op       16 B/op          2 allocs/op
PASS
okJiang commented 2 weeks ago

Seem lots of codes used json.Unmarshal in Clone 🤔 ref https://github.com/tikv/pd/pull/5480

okJiang commented 2 weeks ago

json.Unmarshal and Marshal can indeed conveniently implement Clone without needing to worry about pointer types within the struct. But compared to simple assignment, it does take a bit more time.

Perhaps we should avoid using the Unmarshal/Marshal method for simple structs and keep it unchanged for complex ones.

okJiang commented 2 weeks ago

Just did some bench testing on a simple structure, and found that the performance difference isn't that big...

func BenchmarkCloneWithJSON(b *testing.B) {
    c := &RaftCluster{
        meta: &metapb.Cluster{
            Id:           1,
            MaxPeerCount: 3,
        },
    }
    b.ResetTimer()

    for i := 0; i < b.N; i++ {
        _ = func() *metapb.Cluster {
            return typeutil.DeepClone(c.meta, core.ClusterFactory)
        }
    }
}

func BenchmarkCloneWithAssignment(b *testing.B) {
    c := &RaftCluster{
        meta: &metapb.Cluster{
            Id:           1,
            MaxPeerCount: 3,
        },
    }
    b.ResetTimer()

    for i := 0; i < b.N; i++ {
        _ = func() *metapb.Cluster {
            clone := *c.meta
            return &clone
        }
    }
}

BenchmarkCloneWithJSON-8            1000000000           0.3143 ns/op          0 B/op          0 allocs/op
BenchmarkCloneWithAssignment-8      1000000000           0.3142 ns/op          0 B/op          0 allocs/op