FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

Added UTF-8 validation by Nostimo · Pull Request #13 · simdjson/simdjson-java · GitHub

/ simdjson-java Public

Added UTF-8 validation - #13

Merged
piotrrzysko merged 7 commits into
simdjson:mainfrom
Nostimo:main
Nov 26, 2023
ConversationCommits (7)ChecksFiles changed
Merged

Added UTF-8 validation#13
piotrrzysko merged 7 commits into
simdjson:mainfrom
Nostimo:main

Conversation

Nostimo commented Aug 10, 2023

Copy link
Copy Markdown
Contributor

I've put the utf8 validation in a stage zero method rather than the step method as it was significantly degrading the performance. The performance degradation seemed to be coming from having state, such as storing the previous vector or errors. Hopefully this will improve as the vector API comes out of incubation.

JMH benchmarks before change:

benchmark Throughput (ops/s)
ParseAndSelectBenchmark.countUniqueUsersWithDefaultProfile_fastjson 619.632
ParseAndSelectBenchmark.countUniqueUsersWithDefaultProfile_jackson 472.165
ParseAndSelectBenchmark.countUniqueUsersWithDefaultProfile_jsoniter_scala 1418.109
ParseAndSelectBenchmark.countUniqueUsersWithDefaultProfile_simdjson 1440.042
ParseAndSelectBenchmark.countUniqueUsersWithDefaultProfile_simdjsonPadded 1521.377
ParseBenchmark.simdjson /twitter.json 1535.270
ParseBenchmark.simdjson /github_events.json 17744.694
ParseBenchmark.simdjsonPadded /twitter.json 1685.552
ParseBenchmark.simdjsonPadded /github_events.json 18544.664

After:

benchmark Throughput (ops/s)
ParseAndSelectBenchmark.countUniqueUsersWithDefaultProfile_fastjson 617.064
ParseAndSelectBenchmark.countUniqueUsersWithDefaultProfile_jackson 478.122
ParseAndSelectBenchmark.countUniqueUsersWithDefaultProfile_jsoniter_scala 1431.893
ParseAndSelectBenchmark.countUniqueUsersWithDefaultProfile_simdjson 1311.505
ParseAndSelectBenchmark.countUniqueUsersWithDefaultProfile_simdjsonPadded 1303.818
ParseBenchmark.simdjson /twitter.json 1428.842
ParseBenchmark.simdjson /github_events.json 16368.646
ParseBenchmark.simdjsonPadded /twitter.json 1458.352
ParseBenchmark.simdjsonPadded /github_events.json 16739.397

Additional benchmarks of the SIMD utf8 validation vs utf8 validation from guava:

benchmark Throughput (ops/s)
Utf8ValidatorBenchmark.guava /twitter.json 3802.453
Utf8ValidatorBenchmark.guava /gsoc-2018.json 1420.639
Utf8ValidatorBenchmark.guava /github_events.json 71402.918
Utf8ValidatorBenchmark.utf8Validator /twitter.json 16678.892
Utf8ValidatorBenchmark.utf8Validator /gsoc-2018.json 10601.135
Utf8ValidatorBenchmark.utf8Validator /github_events.json 185121.703

piotrrzysko commented Aug 12, 2023

Copy link
Copy Markdown
Member

Thanks for the PR. I'll take a look at it soon.

piotrrzysko reviewed Aug 13, 2023
View reviewed changes

piotrrzysko left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

I've done the first pass through the PR and left a few comments. I'm going to dive deeper into the implementation next week.

Regarding the performance degradation you mentioned in the PR description:

I've put the utf8 validation in a stage zero method rather than the step method as it was significantly degrading the performance. The performance degradation seemed to be coming from having state, such as storing the previous vector or errors.

I've verified this, and indeed, plugging the validator into StructuralIndexer::step causes performance degradation. In the compilation logs, I've seen that the JIT struggles with inlining Utf8Validator::validate when it's called from the step method, which could potentially be the cause of the performance drop.

Comment thread src/test/java/org/simdjson/Utf8ValidatorTest.java
Comment thread src/test/java/org/simdjson/Utf8ValidatorTest.java Outdated
Comment thread src/main/java/org/simdjson/Utf8Validator.java Outdated
Comment thread src/main/java/org/simdjson/Utf8Validator.java Outdated
Comment thread src/main/java/org/simdjson/Utf8Validator.java Outdated
Comment thread src/jmh/java/org/simdjson/Utf8ValidatorBenchmark.java Outdated

piotrrzysko commented Sep 4, 2023 •
edited
Loading

Copy link
Copy Markdown
Member

Hi @Nostimo, just to let you know: I remember about this PR, but I've been busy with other tasks. I'll try to do the second pass as soon as I can.

piotrrzysko commented Nov 6, 2023

Copy link
Copy Markdown
Member

I’m getting back to this. First, I need to read the paper to properly review the PR.

My plan is as follows:

  • Investigate if we can improve the current performance of the validator. Recently, support for 512-bit vectors has been added to the parser. It would be great to add it to the validator as well. The question is: would you like to do it? If you don't have time, I can work on it.
  • If we can't improve the performance, I think we should consider adding an option to disable the validator. Perhaps, in some cases, validation is unnecessary. For example, when we are certain that the data we are processing is already validated.

piotrrzysko requested changes Nov 12, 2023
View reviewed changes

piotrrzysko left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

@Nostimo I've left a few additional comments. Overall, you did a great job! Thank you for that.

Comment thread src/main/java/org/simdjson/Utf8Validator.java Outdated
Comment thread src/main/java/org/simdjson/Utf8Validator.java Outdated
Comment thread src/main/java/org/simdjson/Utf8Validator.java Outdated
Comment thread src/main/java/org/simdjson/Utf8Validator.java
Comment thread src/test/java/org/simdjson/Utf8ValidatorTest.java Outdated
Comment thread src/test/java/org/simdjson/Utf8ValidatorTest.java Outdated
Comment thread src/test/java/org/simdjson/Utf8ValidatorTest.java Outdated
Comment thread src/test/java/org/simdjson/Utf8ValidatorTest.java

piotrrzysko commented Nov 26, 2023

Copy link
Copy Markdown
Member

Thank you, @Nostimo, for your contribution. I'm merging it. I'll add support for 512-bit vectors in a follow-up PR.

piotrrzysko merged commit bc5e14d into simdjson:main Nov 26, 2023
piotrrzysko mentioned this pull request Nov 26, 2023
Closed
plokhotnyuk mentioned this pull request Nov 26, 2023
Merged
piotrrzysko mentioned this pull request Nov 27, 2023
Merged
Squiry pushed a commit to Squiry/simdjson-java that referenced this pull request Feb 29, 2024
Added UTF-8 validation (simdjson#13)
0d265d2
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

piotrrzysko piotrrzysko requested changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

Back | FazBrowse Home | New Git URL