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

NIOFileHandle: don't read from the file to fill a trash buffer #34

Merged
merged 1 commit into from
Nov 12, 2018

Conversation

popiel
Copy link

@popiel popiel commented Oct 10, 2018

The writeSetup method in NIOFileHandle ensures that the buffer used for speeding up reads is aligned sanely with any write that we're going to do, so that we can also use it as a write-through cache. In the process, it may resize the buffer and read into it from the file on disk. However, the write method taking a ByteBuffer immediately throws away that read buffer, so resizing it and filling it is simply a waste of time.

This work was motivated by the desire to speed up image format conversions for image files in excess of 50GB. In conjunction with additional changes in bioformats and ome-codecs, I have achieved a geometric to linear time asymptotic improvement, plus another ~30% speed improvement of just the linear time cost. The patch for ome-codecs has already been submitted, and the patches for bioformats will be forthcoming in the next few days.

@dgault
Copy link
Member

dgault commented Oct 11, 2018

Thanks for opening this @popiel, the logic certainly seems to make sense in that the write with ByteBuffer is throwing away the buffer after so there is no need for the setup in this instance. I will continue testing and profiling the improvements but on initial tests it is providing a good performance boost.

@sbesson
Copy link
Member

sbesson commented Nov 12, 2018

Thanks @popiel. Merging for inclusion in upcoming milestone of Bio-Formats 6. As mentioned by David above we will carry on profiling the writing of files across various Bio-Formats tools.

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.

3 participants