Skip to content

[client-v2] reader buffer optimization#1816

Merged
chernser merged 6 commits intomainfrom
clientv2_buffer_optimization
Sep 10, 2024
Merged

[client-v2] reader buffer optimization#1816
chernser merged 6 commits intomainfrom
clientv2_buffer_optimization

Conversation

@chernser
Copy link
Contributor

@chernser chernser commented Sep 9, 2024 •
edited
Loading

Summary

A lot small buffers may be allocated while reading numeric information.
This PR implements a reusing buffers withing a reader. It is not a default behavior.
Introduced Client.Builder#allowBinaryReaderToReuseBuffers to control the behavior

Note:

  • implementation is not thread-safe
  • reader should be called by one thread at a time
  • small buffers will 64 bytes are pre-allocated

Checklist

Delete items not relevant to your PR:

  • Unit and integration tests covering the common scenarios were added
  • A human-readable description of the changes was provided to include in CHANGELOG
  • For significant changes, documentation in https://github.com/ClickHouse/clickhouse-docs was updated with further explanations or tutorials

@chernser chernser added this to the 0.7.0 milestone Sep 9, 2024
@sonarqubecloud
Copy link

sonarqubecloud bot commented Sep 9, 2024

Quality Gate Failed Quality Gate failed

Failed conditions
60.5% Coverage on New Code (required ≥ 80%)

See analysis details on SonarCloud

mzitnik
mzitnik reviewed Sep 10, 2024
View reviewed changes
Copy link
Contributor

@mzitnik mzitnik left a comment

Choose a reason for hiding this comment

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

I do not see tests that enable the flag client_allow_binary_reader_to_reuse_buffers

@chernser
Copy link
Contributor Author

chernser commented Sep 10, 2024

I do not see tests that enable the flag client_allow_binary_reader_to_reuse_buffers

I've added client-v2/src/test/java/com/clickhouse/client/query/BinaryReadyReusesBuffersTests.java that overrides QueryTests and sets client_allow_binary_reader_to_reuse_buffers

@chernser chernser requested a review from mzitnik September 10, 2024 07:39
@chernser chernser merged commit dfb161b into main Sep 10, 2024
@chernser chernser deleted the clientv2_buffer_optimization branch September 10, 2024 08:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mzitnik mzitnik mzitnik approved these changes

@Paultagoras Paultagoras Awaiting requested review from Paultagoras

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

0.7.0

Development

Successfully merging this pull request may close these issues.

2 participants

@chernser @mzitnik