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

Test, benchmarks & fixes to UTF-8 validation #752

Merged
merged 5 commits into from
Mar 10, 2021
Merged

Test, benchmarks & fixes to UTF-8 validation #752

merged 5 commits into from
Mar 10, 2021

Conversation

chfast
Copy link
Collaborator

@chfast chfast commented Mar 10, 2021

Includes 32-bit architecture and dead store fixes.
Also added "UTF-8 encoded sample plain-text file" by Markus Kuhn. Used to testing and benchmarking.

parse_string/16_mean                    +0.0013         +0.0013            31            31            31            31
parse_string/32_mean                    +0.0007         +0.0007            30            30            30            30
parse_string/64_mean                    +0.0016         +0.0016            47            47            47            47
parse_string/128_mean                   +0.0003         +0.0003            68            68            68            68
parse_string/256_mean                   +0.0000         +0.0000           109           109           109           109
parse_string/512_mean                   -0.0003         -0.0003           192           192           192           192
parse_string/1024_mean                  +0.0001         +0.0001           362           362           362           362
parse_string/2048_mean                  -0.0009         -0.0009           720           719           720           719
parse_string/4096_mean                  -0.0992         -0.0992          1576          1420          1576          1420
utf8_demo_mean                          +0.0099         +0.0099         16991         17159         16991         17159

@chfast chfast requested review from axic and gumb0 March 10, 2021 09:11
@codecov
Copy link

codecov bot commented Mar 10, 2021

Codecov Report

Merging #752 (9d91eca) into master (56ffca7) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master     #752   +/-   ##
=======================================
  Coverage   99.26%   99.26%           
=======================================
  Files          74       74           
  Lines       11439    11441    +2     
=======================================
+ Hits        11355    11357    +2     
  Misses         84       84           
Flag Coverage Δ
rust 99.86% <ø> (ø)
spectests 90.67% <100.00%> (-0.01%) ⬇️
unittests 99.22% <100.00%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
lib/fizzy/utf8.cpp 100.00% <100.00%> (ø)
test/unittests/utf8_test.cpp 100.00% <100.00%> (ø)


namespace fizzy::test
{
const std::string_view utf8_demo =
Copy link
Member

Choose a reason for hiding this comment

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

Isn't string_view a reference?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It is, but reference to a literal is fine.

@@ -0,0 +1,222 @@
// The content of the "UTF-8 encoded sample plain-text file" by Markus Kuhn.
Copy link
Member

Choose a reason for hiding this comment

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

Do you have a link?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I could not find the original file, but the content has license id and link to author's home page. Many projects uses this file, including:

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@@ -320,7 +320,7 @@ jobs:
- run:
name: "Run codespell"
command: |
codespell --quiet-level=4 -I .codespell-whitelist
codespell --quiet-level=4 --ignore-words .codespell-whitelist --skip 'utf8_demo.cpp'
Copy link
Member

Choose a reason for hiding this comment

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

What does --ignore-words do?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It's expanded -I option.

@axic axic merged commit a2f9b99 into master Mar 10, 2021
@axic axic deleted the utf8 branch March 10, 2021 14:42
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.

3 participants