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

convert pointer to encode #289

Merged
merged 1 commit into from
Nov 16, 2021

Conversation

binbin0325
Copy link
Member

@binbin0325 binbin0325 commented Nov 15, 2021

What this PR does:

When a user implements custom POJOs and uses non-pointer POJOs to encode, the encoding fails to transfer pojos forcibly. As a result, the program execution fails to meet expectations

image

When non-pointer structures are encoded, they are uniformly converted to Pointers for encoding


Which issue(s) this PR fixes:

Fixes #

Special notes for your reviewer:

Does this PR introduce a user-facing change?:


Benchmark: 分别在struct以及pointer场景下 用串行以及8c并行 对当前pr与master进行对比。

// Benchmark_Struct_Encode 2231869 506.8 ns/op 560 B/op 7 allocs/op
// Benchmark_Struct_Encode-master 713761 1549 ns/op 560 B/op 8 allocs/op

// Benchmark_Pointer_Encode 2565778 476.1 ns/op 560 B/op 7 allocs/op
// Benchmark_Pointer_Encode-master 2328867 511.4 ns/op 576 B/op 8 allocs/op

// Benchmark_Struct_Encode_8 2307214 519.4 ns/op 560 B/op 7 allocs/op
// Benchmark_Struct_Encode_8-master 3104438 387.6 ns/op 560 B/op 8 allocs/op

// Benchmark_Pointer_Encode_8 2460842 476.7 ns/op 560 B/op 7 allocs/op
// Benchmark_Pointer_Encode_8-master 2254734 514.9 ns/op 576 B/op 8 allocs/op

当前master 对于struct 是非指针并且receiver是指针的情况下 性能和当前pr差距较大,其他的情况差距都还好。

@codecov-commenter
Copy link

codecov-commenter commented Nov 15, 2021

Codecov Report

Merging #289 (948e953) into master (3a4ef0b) will decrease coverage by 0.18%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #289      +/-   ##
==========================================
- Coverage   70.26%   70.07%   -0.19%     
==========================================
  Files          27       27              
  Lines        3033     3041       +8     
==========================================
  Hits         2131     2131              
- Misses        677      683       +6     
- Partials      225      227       +2     
Impacted Files Coverage Δ
codec.go 69.51% <100.00%> (+0.50%) ⬆️
encode.go 78.26% <100.00%> (+0.58%) ⬆️
serialize.go 66.66% <100.00%> (ø)
java_sql_time.go 61.11% <0.00%> (-5.56%) ⬇️
ref.go 83.78% <0.00%> (-2.71%) ⬇️
object.go 65.69% <0.00%> (-0.59%) ⬇️
pojo.go 64.11% <0.00%> (+0.17%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3a4ef0b...948e953. Read the comment docs.

@wongoo
Copy link
Contributor

wongoo commented Nov 15, 2021

@sanxun0325 I think it's definition issue, if define JavaClassName on struct pointer , then not encode the non-pointer struct instance.

If u want encode the non-pointer struct instance, define JavaClassName on the struct directly.

@binbin0325 binbin0325 force-pushed the encode_convers_pointer_211115 branch from 146bb9b to 948e953 Compare November 16, 2021 01:48
@binbin0325 binbin0325 force-pushed the encode_convers_pointer_211115 branch from 948e953 to 6709afb Compare November 16, 2021 02:08
Copy link
Contributor

@wongoo wongoo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's a compatibility feature to encode instance of non-pointer type, who is not POJO, while its pointer type is POJO.

@wongoo wongoo requested review from justxuewei and zouyx November 16, 2021 03:01
@wongoo wongoo merged commit 57240d6 into apache:master Nov 16, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants