You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@dubbo.apache.org by "nuclearg (via GitHub)" <gi...@apache.org> on 2025/03/31 15:31:32 UTC
[PR] bugfix: decode prefermance fix [dubbo-go-hessian2]
nuclearg opened a new pull request, #375:
URL: https://github.com/apache/dubbo-go-hessian2/pull/375
<!-- Thanks for sending a pull request!
-->
**What this PR does**:
decode.go 里面的 Decode() 每次在解析map或list时都会触发一次对 d.refHolders 的轮询 notify()，在解析复杂对象时（我的场景需要处理约10M的数据），这个 Decode() 会被疯狂递归调用，无数次对这个 d.refHolders() 进行无用的 notify()，产生严重的性能问题
我的机器是 MacBook Pro M3Pro 36G，在我本机上，处理10M的数据时，rt将达到5秒。此性能不可接受。
本次改动对 Decode() 的递归次数进行计数，只在最外层的 Decode() 时进行 notify() 操作。
可以通过MR中的 decode_largedata_test.go 验证。此测试用例随机生成一个巨大的map/list相互嵌套的结构并执行序列化/反序列化操作。未引入我的修改时，反序列化的rt远高于序列化（秒级）。引入此修改后序列化与反序列化rt持平（毫秒级）
**Which issue(s) this PR fixes**:
<!--
*Automatically closes linked issue when PR is merged.
Usage: `Fixes #<issue number>`, or `Fixes (paste link of issue)`.
_If PR is about `failing-tests or flakes`, please post the related issues/tests in a comment and do not use `Fixes`_*
-->
Fixes #
**Special notes for your reviewer**:
**Does this PR introduce a user-facing change?**:
<!--
If no, just write "NONE" in the release-note block below.
If yes, a release note is required:
Enter your extended release note in the block below. If the PR requires additional action from users switching to the new release, include the string "action required".
-->
```release-note
NONE
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
Re: [PR] bugfix: decode prefermance issue fix [dubbo-go-hessian2]
Posted by "AlexStocks (via GitHub)" <gi...@apache.org>.
AlexStocks commented on code in PR #375:
URL: https://github.com/apache/dubbo-go-hessian2/pull/375#discussion_r2174087332
##########
decode_benchmark_test.go:
##########
@@ -0,0 +1,141 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package hessian
+
+import (
+ "fmt"
+ "math/rand"
+ "testing"
+ "time"
+)
+
+func TestMultipleLevelRecursiveDep(t *testing.T) {
+ // ensure encode() and decode() are consistent
+ data := generateLargeMap(2, 10) // about 1M
+
+ encoder := NewEncoder()
+ err := encoder.Encode(data)
+ if err != nil {
+ panic(err)
+ }
+ bytes := encoder.Buffer()
+
+ decoder := NewDecoder(bytes)
+ obj, err := decoder.Decode()
+ if err != nil {
+ panic(err)
+ }
+
+ s1 := fmt.Sprintf("%v", obj)
+ s2 := fmt.Sprintf("%v", data)
+ if s1 != s2 {
+ t.Error("deserialize mismatched")
+ }
+}
+
+func TestMultipleLevelRecursiveDep2(t *testing.T) {
+ // ensure decode() a large object is fast
+ data := generateLargeMap(3, 5) // about 10MB
+
+ now := time.Now()
+
+ encoder := NewEncoder()
+ err := encoder.Encode(data)
+ if err != nil {
+ panic(err)
+ }
+ bytes := encoder.Buffer()
+ fmt.Printf("hessian2 serialize %s %dKB\n", time.Since(now), len(bytes)/1024)
+
+ now = time.Now()
+ decoder := NewDecoder(bytes)
+ obj, err := decoder.Decode()
+ if err != nil {
+ panic(err)
+ }
+ rt := time.Since(now)
+ fmt.Printf("hessian2 deserialize %s\n", rt)
+
+ if rt > 1*time.Second {
+ t.Error("deserialize too slow")
Review Comment:
@nuclearg any feedback?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
Re: [PR] bugfix: decode prefermance issue fix [dubbo-go-hessian2]
Posted by "AlexStocks (via GitHub)" <gi...@apache.org>.
AlexStocks commented on code in PR #375:
URL: https://github.com/apache/dubbo-go-hessian2/pull/375#discussion_r2048046046
##########
decode_benchmark_test.go:
##########
@@ -0,0 +1,140 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package hessian
+
+import (
+ "fmt"
+ "math/rand"
+ "testing"
+ "time"
+)
+
+func TestMultipleLevelRecursiveDep(t *testing.T) {
+ // 测试 encode 和 decode 后的对象和源对象结构是一致的，值是相等的
+ data := generateLargeMap(2, 10) // about 1M
+
+ encoder := NewEncoder()
+ err := encoder.Encode(data)
+ if err != nil {
+ panic(err)
+ }
+ bytes := encoder.Buffer()
+
+ decoder := NewDecoder(bytes)
+ obj, err := decoder.Decode()
+ if err != nil {
+ panic(err)
+ }
+
+ s1 := fmt.Sprintf("%v", obj)
+ s2 := fmt.Sprintf("%v", data)
+ if s1 != s2 {
+ t.Error("deserialize mismatched")
+ }
+}
+
+func TestMultipleLevelRecursiveDep2(t *testing.T) {
+ data := generateLargeMap(3, 5) // about 10MB
+
+ now := time.Now()
+
+ encoder := NewEncoder()
+ err := encoder.Encode(data)
+ if err != nil {
+ panic(err)
+ }
+ bytes := encoder.Buffer()
+ fmt.Printf("hessian2 serialize %s %dKB\n", time.Since(now), len(bytes)/1024)
+
+ now = time.Now()
+ decoder := NewDecoder(bytes)
+ obj, err := decoder.Decode()
+ if err != nil {
+ panic(err)
+ }
+ rt := time.Since(now)
+ fmt.Printf("hessian2 deserialize %s\n", rt)
+
+ if rt > 1*time.Second {
+ t.Error("deserialize too slow")
+ }
+ s1 := fmt.Sprintf("%v", obj)
+ s2 := fmt.Sprintf("%v", data)
+ if s1 != s2 {
+ t.Error("deserialize mismatched")
+ }
+}
+func BenchmarkMultipleLevelRecursiveDep(b *testing.B) {
+ // 构造一个多层循环依赖的对象, 无需大对象， 压测情况大批量请求可以看出是否有性能改进
Review Comment:
do not use Chinese comment. Please translate it into English.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
Re: [PR] bugfix: decode prefermance issue fix [dubbo-go-hessian2]
Posted by "AlexStocks (via GitHub)" <gi...@apache.org>.
AlexStocks commented on PR #375:
URL: https://github.com/apache/dubbo-go-hessian2/pull/375#issuecomment-2831866053
@nuclearg please fix the ci failure
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
Re: [PR] bugfix: decode prefermance issue fix [dubbo-go-hessian2]
Posted by "AlexStocks (via GitHub)" <gi...@apache.org>.
AlexStocks commented on PR #375:
URL: https://github.com/apache/dubbo-go-hessian2/pull/375#issuecomment-2862940834
@nuclearg please fix the ci failure.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
Re: [PR] bugfix: decode prefermance issue fix [dubbo-go-hessian2]
Posted by "wongoo (via GitHub)" <gi...@apache.org>.
wongoo commented on code in PR #375:
URL: https://github.com/apache/dubbo-go-hessian2/pull/375#discussion_r2029678188
##########
decode_largedata_test.go:
##########
@@ -0,0 +1,93 @@
+package hessian
Review Comment:
license header needed
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
Re: [PR] bugfix: decode prefermance issue fix [dubbo-go-hessian2]
Posted by "tiltwind (via GitHub)" <gi...@apache.org>.
tiltwind commented on code in PR #375:
URL: https://github.com/apache/dubbo-go-hessian2/pull/375#discussion_r2101562677
##########
decode_benchmark_test.go:
##########
@@ -0,0 +1,141 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package hessian
+
+import (
+ "fmt"
+ "math/rand"
+ "testing"
+ "time"
+)
+
+func TestMultipleLevelRecursiveDep(t *testing.T) {
+ // ensure encode() and decode() are consistent
+ data := generateLargeMap(2, 10) // about 1M
+
+ encoder := NewEncoder()
+ err := encoder.Encode(data)
+ if err != nil {
+ panic(err)
+ }
+ bytes := encoder.Buffer()
+
+ decoder := NewDecoder(bytes)
+ obj, err := decoder.Decode()
+ if err != nil {
+ panic(err)
+ }
+
+ s1 := fmt.Sprintf("%v", obj)
+ s2 := fmt.Sprintf("%v", data)
+ if s1 != s2 {
+ t.Error("deserialize mismatched")
+ }
+}
+
+func TestMultipleLevelRecursiveDep2(t *testing.T) {
+ // ensure decode() a large object is fast
+ data := generateLargeMap(3, 5) // about 10MB
+
+ now := time.Now()
+
+ encoder := NewEncoder()
+ err := encoder.Encode(data)
+ if err != nil {
+ panic(err)
+ }
+ bytes := encoder.Buffer()
+ fmt.Printf("hessian2 serialize %s %dKB\n", time.Since(now), len(bytes)/1024)
+
+ now = time.Now()
+ decoder := NewDecoder(bytes)
+ obj, err := decoder.Decode()
+ if err != nil {
+ panic(err)
+ }
+ rt := time.Since(now)
+ fmt.Printf("hessian2 deserialize %s\n", rt)
+
+ if rt > 1*time.Second {
+ t.Error("deserialize too slow")
Review Comment:
the CI failed
```
=== RUN TestMultipleLevelRecursiveDep2
hessian2 serialize 3.948985828s 10660KB
hessian2 deserialize 1.218173434s
decode_benchmark_test.go:75: deserialize too slow
--- FAIL: TestMultipleLevelRecursiveDep2 (11.67s)
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
Re: [PR] bugfix: decode prefermance issue fix [dubbo-go-hessian2]
Posted by "AlexStocks (via GitHub)" <gi...@apache.org>.
AlexStocks commented on code in PR #375:
URL: https://github.com/apache/dubbo-go-hessian2/pull/375#discussion_r2048041106
##########
decode_benchmark_test.go:
##########
@@ -0,0 +1,140 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package hessian
+
+import (
+ "fmt"
+ "math/rand"
+ "testing"
+ "time"
+)
+
+func TestMultipleLevelRecursiveDep(t *testing.T) {
+ // 测试 encode 和 decode 后的对象和源对象结构是一致的，值是相等的
Review Comment:
do not use Chinese comment. Please translate it into English.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
Re: [PR] bugfix: decode prefermance issue fix [dubbo-go-hessian2]
Posted by "wongoo (via GitHub)" <gi...@apache.org>.
wongoo commented on code in PR #375:
URL: https://github.com/apache/dubbo-go-hessian2/pull/375#discussion_r2029678308
##########
decode_largedata_test.go:
##########
@@ -0,0 +1,93 @@
+package hessian
+
+import (
+ "encoding/json"
+ "fmt"
+ "math/rand"
+ "testing"
+ "time"
+)
+
+func TestDecodeLargeData(t *testing.T) {
Review Comment:
change the test to Benchmark instead
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
Re: [PR] bugfix: decode prefermance issue fix [dubbo-go-hessian2]
Posted by "nuclearg (via GitHub)" <gi...@apache.org>.
nuclearg commented on PR #375:
URL: https://github.com/apache/dubbo-go-hessian2/pull/375#issuecomment-2808486376
@tiltwind 基准测试用例已提交
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
Re: [PR] bugfix: decode prefermance issue fix [dubbo-go-hessian2]
Posted by "AlexStocks (via GitHub)" <gi...@apache.org>.
AlexStocks merged PR #375:
URL: https://github.com/apache/dubbo-go-hessian2/pull/375
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
Re: [PR] bugfix: decode prefermance issue fix [dubbo-go-hessian2]
Posted by "tiltwind (via GitHub)" <gi...@apache.org>.
tiltwind commented on PR #375:
URL: https://github.com/apache/dubbo-go-hessian2/pull/375#issuecomment-2788957022
@nuclearg 感谢
1. 有一个错误要解决
```
Error: decode_benchmark_test.go:54:2: ineffectual assignment to `now` (ineffassign)
now = time.Now()
```
2. 测试和压测可以考虑改一下：
```
func TestMultipleLevelRecursiveDep(t *testing.T) {
// TODO : 测试 encode 和 decode 后的对象和源对象结构是一致的，值是相等的
}
func BenchmarkMultipleLevelRecursiveDep(b *testing.B) {
// TODO: 构造一个多层循环依赖的对象, 无需大对象， 压测情况大批量请求可以看出是否有性能改进
for i := 0; i < b.N; i++ {
// TODO : 循环 encode 和 decode
}
}
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
Re: [PR] bugfix: decode prefermance issue fix [dubbo-go-hessian2]
Posted by "wongoo (via GitHub)" <gi...@apache.org>.
wongoo commented on code in PR #375:
URL: https://github.com/apache/dubbo-go-hessian2/pull/375#discussion_r2029679213
##########
decode_largedata_test.go:
##########
@@ -0,0 +1,93 @@
+package hessian
+
+import (
+ "encoding/json"
+ "fmt"
+ "math/rand"
+ "testing"
+ "time"
+)
+
+func TestDecodeLargeData(t *testing.T) {
Review Comment:
I think this pr is a general performance improvement , it only needs a simple struct decode benchmark to prove that.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org