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

Fix emoji decoding error #254

Merged
merged 4 commits into from
Jan 5, 2021
Merged

Fix emoji decoding error #254

merged 4 commits into from
Jan 5, 2021

Conversation

wongoo
Copy link
Contributor

@wongoo wongoo commented Jan 3, 2021

What this PR does:
fix emoji decoding error and refactor string decoding algorithm, with 17% performance improvement

Which issue(s) this PR fixes:

Fixes #252 #253

Special notes for your reviewer:

Does this PR introduce a user-facing change?:

- fix emoji decoding error and refactor string decoding algorithm, with 17% performance improvement

@wongoo wongoo requested review from zonghaishang, gaoxinge, AlexStocks and fangyincheng and removed request for zonghaishang January 3, 2021 09:44
@wongoo wongoo changed the title Fix emoji Fix emoji decoding error Jan 3, 2021
@codecov-io
Copy link

Codecov Report

Merging #254 (cc4f9a4) into master (4014d7e) will increase coverage by 1.42%.
The diff coverage is 78.04%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #254      +/-   ##
==========================================
+ Coverage   66.19%   67.61%   +1.42%     
==========================================
  Files          25       25              
  Lines        2801     2671     -130     
==========================================
- Hits         1854     1806      -48     
+ Misses        718      644      -74     
+ Partials      229      221       -8     
Impacted Files Coverage Δ
string.go 70.70% <78.04%> (+13.99%) ⬆️
decode.go 72.00% <0.00%> (-2.00%) ⬇️

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 4014d7e...cc4f9a4. Read the comment docs.

@wongoo
Copy link
Contributor Author

wongoo commented Jan 3, 2021

benchmark result

before fixing:

BenchmarkDecodeStringAscii
BenchmarkDecodeStringAscii-8                6952            173795 ns/op
BenchmarkDecodeStringUnicode
BenchmarkDecodeStringUnicode-8              5228            229713 ns/op
BenchmarkDecodeStringEmoji
    string_test.go:230: err: bad utf-8 encoding
--- FAIL: BenchmarkDecodeStringEmoji

after fixing:

BenchmarkDecodeStringAscii
BenchmarkDecodeStringAscii-8                7149            145317 ns/op
BenchmarkDecodeStringUnicode
BenchmarkDecodeStringUnicode-8              5768            182145 ns/op
BenchmarkDecodeStringEmoji
BenchmarkDecodeStringEmoji-8                1400            816080 ns/op

with 17% performance improvement.

@wongoo wongoo requested a review from zouyx January 4, 2021 01:12
@fangyincheng
Copy link
Contributor

ci failed
The command "/tmp/tools/license/license-header-checker -v -a -r -i vendor /tmp/tools/license/license.txt . go && [[ -z `git status -s` ]]" exited with 1.

@AlexStocks AlexStocks merged commit 83d9f69 into master Jan 5, 2021
zhaoyunxing92 pushed a commit that referenced this pull request Sep 4, 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.

decode fail when received string from server,and string contains emoji and chinese
5 participants