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

[PR 5/5] Hide Segment::data #339

Merged
merged 2 commits into from
Aug 23, 2024
Merged

[PR 5/5] Hide Segment::data #339

merged 2 commits into from
Aug 23, 2024

Conversation

fzhinkin
Copy link
Collaborator

@fzhinkin fzhinkin commented Jun 10, 2024

This PR concludes unsafe-API-related changes by hiding the Segment::data.

The change itself has nothing to do with unsafe API as it is now but ensures that direct internal data won't leak into the codebase.

The latter unlocks features like #331 and #239

@fzhinkin fzhinkin marked this pull request as ready for review June 10, 2024 10:24
@fzhinkin fzhinkin requested a review from shanshin June 10, 2024 10:24
core/common/src/Buffer.kt Outdated Show resolved Hide resolved
core/common/src/Buffer.kt Show resolved Hide resolved
core/common/src/Buffer.kt Show resolved Hide resolved
val v = (
data[pos++] and 0xffL shl 56
or (data[pos++] and 0xffL shl 48)
or (data[pos++] and 0xffL shl 40)
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe it's paranoia, but does it work at the same speed as
or (data[pos + 1] and 0xffL shl 40)
?

(there is no frequent updating of the field value)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

pos here is a local variable defined few lines above, so the field will be updated only once, after all reads are done.

Anyway, I'll re-check how it works with explicit offsets and immutable base, the last time I tested it, there was no significant difference.

Base automatically changed from bulk-api-part-3 to develop August 23, 2024 14:10
@fzhinkin fzhinkin merged commit 8a86a68 into develop Aug 23, 2024
1 check passed
@fzhinkin fzhinkin deleted the bulk-api-part-4 branch August 23, 2024 14:33
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.

2 participants