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

IOHasBufFS interface for I/O using user-supplied buffers #46

Merged
merged 1 commit into from
Mar 14, 2024

Conversation

jorisdral
Copy link
Collaborator

@jorisdral jorisdral commented Feb 28, 2024

We require this interface in https://github.com/input-output-hk/lsm-tree, because going through ByteString is incompatible with the code in some places, and it's costly to allocate intermediate ByteStrings instead of moving around bytes from/to Ptrs (and ByteArrays or MutableByteArrays)

Two notes:

  • It's a separated interface from HasFS (for now), since I don't want to break downstream code yet.
  • The interface is not yet simulated and extensively tested using state-machine tests in fs-sim, but will be after a future PR that I'm already working on. I'm currently hunting a persistent bug in the pure model used for the state-machine tests. For now, I've included a couple of simple round-trip tests that exercise the new functions

@jorisdral jorisdral self-assigned this Feb 28, 2024
@jorisdral jorisdral force-pushed the jdral/preadbuf-preadwrite branch 3 times, most recently from 2fa9bf4 to acc91a6 Compare March 6, 2024 13:06
@jorisdral jorisdral changed the title ioHasBufFS IOHasBufFS interface for I/O using user-supplied buffers Mar 6, 2024
@jorisdral jorisdral added the enhancement New feature or request label Mar 6, 2024
@jorisdral jorisdral requested a review from dcoutts March 6, 2024 13:07
@jorisdral jorisdral force-pushed the jdral/preadbuf-preadwrite branch from acc91a6 to 6f17aa7 Compare March 6, 2024 13:12
@jorisdral jorisdral marked this pull request as ready for review March 6, 2024 13:17
@jorisdral jorisdral requested a review from a team as a code owner March 6, 2024 13:17
fs-api/src/System/FS/API.hs Outdated Show resolved Hide resolved
@jorisdral jorisdral marked this pull request as draft March 7, 2024 12:46
@jorisdral
Copy link
Collaborator Author

Marked as draft again because of #46 (comment)

@jorisdral jorisdral force-pushed the jdral/preadbuf-preadwrite branch 11 times, most recently from 4d894a5 to 6438163 Compare March 8, 2024 16:55
@jorisdral
Copy link
Collaborator Author

Alright, ByteArray instead of Ptr seems the way to go. See the changelog file for a full overview of user-facing changes.

@jorisdral jorisdral marked this pull request as ready for review March 8, 2024 16:56
@jorisdral
Copy link
Collaborator Author

Drafting this again, since we're planning on using MutableByteArray instead of ByteArray as the buffer type

@jorisdral jorisdral marked this pull request as draft March 12, 2024 22:34
@jorisdral jorisdral marked this pull request as ready for review March 12, 2024 23:26
@jorisdral jorisdral marked this pull request as draft March 12, 2024 23:26
@jorisdral jorisdral changed the base branch from main to jdral/hTell March 12, 2024 23:27
@jorisdral jorisdral changed the base branch from jdral/hTell to main March 12, 2024 23:27
@jorisdral jorisdral marked this pull request as ready for review March 12, 2024 23:31
@jorisdral jorisdral marked this pull request as draft March 12, 2024 23:31
@jorisdral jorisdral force-pushed the jdral/preadbuf-preadwrite branch from 6438163 to ffceeae Compare March 13, 2024 01:48
@jorisdral jorisdral changed the base branch from main to jdral/hTell March 13, 2024 15:28
@jorisdral jorisdral changed the base branch from jdral/hTell to main March 13, 2024 15:28
@jorisdral jorisdral force-pushed the jdral/preadbuf-preadwrite branch 4 times, most recently from 335c12a to 44675ec Compare March 13, 2024 16:29
@jorisdral jorisdral marked this pull request as ready for review March 13, 2024 16:39
Copy link

@dcoutts dcoutts left a comment

Choose a reason for hiding this comment

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

Looks good. In particular the new API looks good.

fs-api/CHANGELOG.md Outdated Show resolved Hide resolved
fs-api/src/System/FS/API.hs Outdated Show resolved Hide resolved
@jorisdral jorisdral force-pushed the jdral/preadbuf-preadwrite branch from 44675ec to 752bb64 Compare March 14, 2024 13:18
@jorisdral jorisdral enabled auto-merge March 14, 2024 13:19
@jorisdral jorisdral disabled auto-merge March 14, 2024 14:41
@jorisdral jorisdral merged commit 6a4a456 into main Mar 14, 2024
18 checks passed
@jorisdral jorisdral deleted the jdral/preadbuf-preadwrite branch March 14, 2024 14:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants