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

ChunkedNioFile can use absolute FileChannel::read to read chunks #9592

Merged
merged 1 commit into from Sep 24, 2019

Conversation

franz1981
Copy link
Contributor

Motivation:

Users can reuse the same FileChannel for different ChunkedNioFile
instances without being worried that FileChannel::position will be
changed concurrently by them.
In addition, FileChannel::read with absolute position allows to
use on *nix pread that is more efficient then fread.

Modifications:

Always use absolute FileChannel::read ops

Result:

Faster and more flexible uses of FileChannel for ChunkedNioFile

@netty-bot
Copy link

Can one of the admins verify this patch?

@franz1981
Copy link
Contributor Author

franz1981 commented Sep 22, 2019

I see that the original version was very naive (mine too) by changing FileChannel::position only when offset!=0.
If a user has already modified FileChannel::position and will call new ChunkedNioFile(in, 0, file.size, 8192), the old implementation was not changing FileChannel::position ie the next read would have started from offset == 0, but actually reading at FileChannel::position != 0.

Copy link
Member

@njhill njhill left a comment

Choose a reason for hiding this comment

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

LGTM

@normanmaurer
Copy link
Member

@netty-bot test this please

Motivation:

Users can reuse the same FileChannel for different ChunkedNioFile
instances without being worried that FileChannel::position will be
changed concurrently by them.
In addition, FileChannel::read with absolute position allows to
use on *nix pread that is more efficient then fread.

Modifications:

Always use absolute FileChannel::read ops

Result:

Faster and more flexible uses of FileChannel for ChunkedNioFile
@normanmaurer
Copy link
Member

@netty-bot test this please

@normanmaurer normanmaurer added this to the 4.1.42.Final milestone Sep 23, 2019
@normanmaurer normanmaurer merged commit eb3c4bd into netty:4.1 Sep 24, 2019
normanmaurer pushed a commit that referenced this pull request Sep 24, 2019
Motivation:

Users can reuse the same FileChannel for different ChunkedNioFile
instances without being worried that FileChannel::position will be
changed concurrently by them.
In addition, FileChannel::read with absolute position allows to
use on *nix pread that is more efficient then fread.

Modifications:

Always use absolute FileChannel::read ops

Result:

Faster and more flexible uses of FileChannel for ChunkedNioFile
@normanmaurer
Copy link
Member

@franz1981 thanks a lot!

Copy link

@wy96f wy96f left a comment

Choose a reason for hiding this comment

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

Good job :)

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.

None yet

5 participants