Skip to content

refactor(inkless): pass bytebuffer around #301

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

jeqo
Copy link
Contributor

@jeqo jeqo commented May 19, 2025

This arguably does not reduce allocation as a buffer still needs to be created to be uploaded and a subset is allocated to be cached; but passing the bytebuffer may allow some further improvements as have resumable inputstreams for remote storage uploads.

This removes the need to allocate the bytes. It passes byteBuffer to uploader, so a ByteBufferInputStream can be used to upload; and passes a byteBuffer to the cache, so a slice array can be passed to the cache.

@jeqo jeqo marked this pull request as ready for review May 19, 2025 14:37
Base automatically changed from jeqo/fixed-thread-pools to jeqo/refactor-append-completer May 22, 2025 08:53
Base automatically changed from jeqo/refactor-append-completer to main May 22, 2025 09:00
@ivanyu
Copy link
Member

ivanyu commented May 22, 2025

@jeqo this seems to need rebasing

This arguably does not reduce allocation as a buffer still needs to be
created to be uploaded and a subset is allocated to be cached; but
passing the bytebuffer may allow some further improvements as have
resumable inputstreams for remote storage uploads.
@jeqo jeqo force-pushed the jeqo/less-allocation branch from ce4b6f0 to 2048356 Compare May 22, 2025 13:31
@jeqo jeqo requested a review from ivanyu May 22, 2025 13:33
@jeqo jeqo changed the title refactor(inkless): avoid alloc by passing bytebuffer around refactor(inkless): pass bytebuffer around May 22, 2025
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