From 219c10bc21da57e70f44c0990ef70bc06f2bd1e1 Mon Sep 17 00:00:00 2001 From: Yan Zhu Date: Sat, 7 Oct 2023 17:42:25 +0800 Subject: [PATCH 1/2] perf: optimize memory usage for client.List func MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit If there is a predicate condition, we should not alloc a very large slice ``` goos: linux goarch: amd64 pkg: github.com/ovn-org/libovsdb/client cpu: Intel(R) Core(TM) i7-1065G7 CPU @ 1.30GHz │ bench.out.main │ bench.out.new │ │ sec/op │ sec/op vs base │ APIList/predicate_returns_none-4 3.419m ± 41% 3.238m ± 1% -5.29% (p=0.001 n=10) APIList/predicate_returns_all-4 67.92m ± 3% 68.95m ± 1% ~ (p=0.123 n=10) APIList/predicate_on_an_arbitrary_condition-4 11.88m ± 2% 12.10m ± 11% ~ (p=0.075 n=10) APIList/predicate_matches_name-4 4.073m ± 4% 5.006m ± 17% +22.91% (p=0.000 n=10) APIList/by_index,_no_predicate-4 32.644µ ± 4% 7.957µ ± 3% -75.62% (p=0.000 n=10) APIListMultiple/multiple_results_one_at_a_time_with_Get-4 6.073m ± 4% 6.021m ± 11% ~ (p=0.853 n=10) APIListMultiple/multiple_results_in_a_batch_with_WhereAny-4 3.014m ± 2% 3.085m ± 19% +2.36% (p=0.002 n=10) geomean 3.521m 2.962m -15.89% │ bench.out.main │ bench.out.new │ │ B/op │ B/op vs base │ APIList/predicate_returns_none-4 908.6Ki ± 0% 828.6Ki ± 0% -8.80% (p=0.000 n=10) APIList/predicate_returns_all-4 14.88Mi ± 0% 14.88Mi ± 0% ~ (p=0.315 n=10) APIList/predicate_on_an_arbitrary_condition-4 2.468Mi ± 0% 2.399Mi ± 0% -2.80% (p=0.000 n=10) APIList/predicate_matches_name-4 910.3Ki ± 0% 830.3Ki ± 0% -8.79% (p=0.000 n=10) APIList/by_index,_no_predicate-4 81.997Ki ± 0% 2.000Ki ± 0% -97.56% (p=0.000 n=10) APIListMultiple/multiple_results_one_at_a_time_with_Get-4 1.439Mi ± 0% 1.439Mi ± 0% ~ (p=0.197 n=10) APIListMultiple/multiple_results_in_a_batch_with_WhereAny-4 712.7Ki ± 0% 712.7Ki ± 0% ~ (p=0.280 n=10) geomean 1.128Mi 659.2Ki -42.93% │ bench.out.main │ bench.out.new │ │ allocs/op │ allocs/op vs base │ APIList/predicate_returns_none-4 20.01k ± 0% 20.01k ± 0% -0.00% (p=0.000 n=10) APIList/predicate_returns_all-4 230.1k ± 0% 230.1k ± 0% +0.00% (p=0.048 n=10) APIList/predicate_on_an_arbitrary_condition-4 43.36k ± 0% 43.36k ± 0% ~ (p=1.000 n=10) APIList/predicate_matches_name-4 20.03k ± 0% 20.03k ± 0% ~ (p=1.000 n=10) ¹ APIList/by_index,_no_predicate-4 32.00 ± 0% 32.00 ± 0% ~ (p=1.000 n=10) ¹ APIListMultiple/multiple_results_one_at_a_time_with_Get-4 22.02k ± 0% 22.01k ± 0% ~ (p=0.170 n=10) APIListMultiple/multiple_results_in_a_batch_with_WhereAny-4 11.51k ± 0% 11.51k ± 0% ~ (p=1.000 n=10) ¹ geomean 11.83k 11.83k -0.00% ¹ all samples are equal ``` Signed-off-by: Yan Zhu --- client/api.go | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/client/api.go b/client/api.go index 8d5a30ef..47cbd770 100644 --- a/client/api.go +++ b/client/api.go @@ -147,25 +147,28 @@ func (a api) List(ctx context.Context, result interface{}) error { return ErrNotFound } - // If given a null slice, fill it in the cache table completely, if not, just up to - // its capability - if resultVal.IsNil() || resultVal.Cap() == 0 { - resultVal.Set(reflect.MakeSlice(resultVal.Type(), 0, tableCache.Len())) - } - i := resultVal.Len() - var rows map[string]model.Model if a.cond != nil { rows, err = a.cond.Matches() if err != nil { return err } + if resultVal.IsNil() || resultVal.Cap() == 0 { + resultVal.Set(reflect.MakeSlice(resultVal.Type(), 0, len(rows))) + } } else { rows = tableCache.Rows() + // If given a null slice, fill it in the cache table completely, if not, just up to + // its capability. + if resultVal.IsNil() || resultVal.Cap() == 0 { + resultVal.Set(reflect.MakeSlice(resultVal.Type(), 0, tableCache.Len())) + } } + i := resultVal.Len() + maxCap := resultVal.Cap() for _, row := range rows { - if i >= resultVal.Cap() { + if i >= maxCap { break } appendValue(reflect.ValueOf(row)) From 776d5b20c5e31639413cbf7e00aeb4dd160567ab Mon Sep 17 00:00:00 2001 From: Yan Zhu Date: Tue, 2 Jan 2024 18:32:26 +0800 Subject: [PATCH 2/2] fix review Signed-off-by: Yan Zhu --- client/api.go | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/client/api.go b/client/api.go index 47cbd770..9c295638 100644 --- a/client/api.go +++ b/client/api.go @@ -153,16 +153,13 @@ func (a api) List(ctx context.Context, result interface{}) error { if err != nil { return err } - if resultVal.IsNil() || resultVal.Cap() == 0 { - resultVal.Set(reflect.MakeSlice(resultVal.Type(), 0, len(rows))) - } } else { rows = tableCache.Rows() - // If given a null slice, fill it in the cache table completely, if not, just up to - // its capability. - if resultVal.IsNil() || resultVal.Cap() == 0 { - resultVal.Set(reflect.MakeSlice(resultVal.Type(), 0, tableCache.Len())) - } + } + // If given a null slice, fill it in the cache table completely, if not, just up to + // its capability. + if resultVal.IsNil() || resultVal.Cap() == 0 { + resultVal.Set(reflect.MakeSlice(resultVal.Type(), 0, len(rows))) } i := resultVal.Len() maxCap := resultVal.Cap()