Skip to content

GH-51471: [C++] Disable simdjson threading in chunker - #51473

Merged
pitrou merged 2 commits into
apache:mainfrom
taepper:GH-51471
Sep 29, 2026
Merged

pitrou merged 2 commits into
apache:mainfrom
taepper:GH-51471

Conversation

@taepper

@taepper taepper commented Sep 23, 2026 •

Copy link
Copy Markdown
Contributor

Rationale for this change

Resolves #51471:

When invoking simdjson's parse_many, it uses threading to perform its stage 1 parsing on the next batch when while parsing the current batch:
https://github.com/simdjson/simdjson/blob/master/doc/parse_many.md?plain=1#L96-L104

In most cases, we do not need to parse any batches after the first one to find delimiters (in fact, we currently would even error, when the first batch does not contain the whole first document). By disabling the parser's threading we can expect performance improvements

What changes are included in this PR?

This disables threading in the parser by setting parser_.threaded = false;

Are these changes tested?

Yes

before (origin/main bb83012743):

ChunkJSONPrettyPrintedMultipleBlocks            187462 ns       158760 ns         4415 block_size=27.344k
bytes_per_second=1.28328Gi/s json_size=218.757k

after (taepper:GH-51471 fb56ccb2a1):

ChunkJSONPrettyPrintedMultipleBlocks             98726 ns        98721 ns         7144 block_size=27.344k
bytes_per_second=2.06374Gi/s json_size=218.757k

Are there any user-facing changes?

No

Was AI used for this PR?

PR code and description written by:

  • Human
  • AI

Reviewed before submission by:

  • Human
  • AI
  • Not reviewed

@github-actions

Copy link
Copy Markdown

⚠️ GitHub issue #51471 has been automatically assigned in GitHub to PR creator.

@taepper

taepper commented Sep 23, 2026 •

Copy link
Copy Markdown
Contributor Author

If you would have time for review, I think this is a good improvement to the chunker @pitrou (or please @ others that might be a good fit to review this)

@pitrou pitrou 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.

LGTM, can you rebase/merge so that we get updated CI?

@github-actions github-actions Bot added awaiting committer review Awaiting committer review and removed awaiting review Awaiting review labels Sep 28, 2026
@taepper

taepper commented Sep 28, 2026

Copy link
Copy Markdown
Contributor Author

LGTM, can you rebase/merge so that we get updated CI?

Thank you for the review, and also #51472! I found these while trying to investigate possible improvements we can achieve with #51463. While studying the docs, I noticed that we definitely still lack some familiarity, and a few simdjson code sections that were recently introduced, use the library in suboptimal ways.

I merged main into the branch to retrigger CI with all latest changes (in particular including #51472)

@pitrou pitrou added the CI: Extra: C++ Run extra C++ CI label Sep 28, 2026
@pitrou

pitrou commented Sep 28, 2026

Copy link
Copy Markdown
Member

@taepper Can you perhaps update the PR description (especially benchmark numbers) now that #51470 has been merged?

@taepper

taepper commented Sep 28, 2026

Copy link
Copy Markdown
Contributor Author

@taepper Can you perhaps update the PR description (especially benchmark numbers) now that #51470 has been merged?

I updated the benchmarks, the PR description is still valid

@pitrou
pitrou merged commit 331b0f5 into apache:main Sep 29, 2026
89 of 93 checks passed
@pitrou pitrou removed the awaiting committer review Awaiting committer review label Sep 29, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[C++] Disable simdjson threading in chunker

2 participants