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] accept tabs as tokens after : and - #211

Merged
merged 5 commits into from
Feb 10, 2022
Merged

[fix] accept tabs as tokens after : and - #211

merged 5 commits into from
Feb 10, 2022

Conversation

biojppm
Copy link
Owner

@biojppm biojppm commented Feb 5, 2022

No description provided.

@codecov
Copy link

codecov bot commented Feb 5, 2022

Codecov Report

Merging #211 (d7d0190) into master (10d68ca) will increase coverage by 0.06%.
The diff coverage is 97.84%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #211      +/-   ##
==========================================
+ Coverage   97.00%   97.06%   +0.06%     
==========================================
  Files          77       77              
  Lines       16672    16664       -8     
==========================================
+ Hits        16172    16175       +3     
+ Misses        500      489      -11     
Impacted Files Coverage Δ
src/c4/yml/detail/parser_dbg.hpp 29.62% <ø> (ø)
src/c4/yml/parse.hpp 100.00% <ø> (ø)
test/test_seq_of_map.cpp 100.00% <ø> (ø)
test/test_simple_map.cpp 98.30% <ø> (ø)
test/test_simple_seq.cpp 97.00% <ø> (ø)
test/test_suite/test_suite_parts.cpp 100.00% <ø> (ø)
src/c4/yml/parse.cpp 95.63% <97.84%> (+0.43%) ⬆️

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 10d68ca...d7d0190. Read the comment docs.

@biojppm
Copy link
Owner Author

biojppm commented Feb 5, 2022

This feature is too costly for the benefits it offers. It costs somewhere between 10-15% in throughput/cpu time because of the added branching.

Here is a benchmark for parsing a file which is not even token-heavy.

First, before the feature:

---------------------------------------------------------------------------------
Benchmark                    Time             CPU   Iterations UserCounters...
---------------------------------------------------------------------------------
ryml_inplace_reuse       12301 ns        12308 ns        56592 bytes_per_second=165.04M/s items_per_second=81.2476k/s
ryml_arena_reuse         12445 ns        12453 ns        55843 bytes_per_second=163.12M/s items_per_second=80.3021k/s
ryml_inplace             14390 ns        14394 ns        49587 bytes_per_second=141.119M/s items_per_second=69.4716k/s
ryml_arena               15190 ns        15183 ns        51004 bytes_per_second=133.792M/s items_per_second=65.8644k/s
libyaml_arena            32065 ns        32052 ns        21070 bytes_per_second=63.3758M/s items_per_second=31.1992k/s
libyaml_arena_reuse      32453 ns        32465 ns        22356 bytes_per_second=62.5702M/s items_per_second=30.8026k/s
libfyaml_arena           45159 ns        45141 ns        15089 bytes_per_second=44.9992M/s items_per_second=22.1526k/s
yamlcpp_arena           235727 ns       235590 ns         3354 bytes_per_second=8.6223M/s items_per_second=4.24467k/s

Now, with the feature:

---------------------------------------------------------------------------------
Benchmark                    Time             CPU   Iterations UserCounters...
---------------------------------------------------------------------------------
ryml_inplace_reuse       13365 ns        13365 ns        50357 bytes_per_second=151.989M/s items_per_second=74.8224k/s
ryml_arena_reuse         13598 ns        13602 ns        51875 bytes_per_second=149.338M/s items_per_second=73.5175k/s
ryml_inplace             15222 ns        15229 ns        45948 bytes_per_second=133.384M/s items_per_second=65.6637k/s
ryml_arena               14721 ns        14716 ns        46684 bytes_per_second=138.032M/s items_per_second=67.9519k/s
libyaml_arena            32124 ns        32113 ns        21813 bytes_per_second=63.2553M/s items_per_second=31.1399k/s
libyaml_arena_reuse      31572 ns        31589 ns        21761 bytes_per_second=64.3045M/s items_per_second=31.6564k/s
libfyaml_arena           45563 ns        45546 ns        15376 bytes_per_second=44.5992M/s items_per_second=21.9557k/s
yamlcpp_arena           211465 ns       211388 ns         3318 bytes_per_second=9.60946M/s items_per_second=4.73063k/s

@biojppm biojppm force-pushed the events branch 3 times, most recently from d7abb96 to 1cedc69 Compare February 9, 2022 15:11
@biojppm biojppm merged commit 185615c into master Feb 10, 2022
@biojppm biojppm deleted the events branch February 10, 2022 02:29
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.

1 participant