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

new sync IO api #2015

Merged
merged 1 commit into from
May 16, 2024
Merged

new sync IO api #2015

merged 1 commit into from
May 16, 2024

Conversation

mikea
Copy link
Collaborator

@mikea mikea commented Apr 29, 2024

This change reworks all synchronoush IO API in terms of ArrayPtr to be safer.

Read API now is:

  size_t read(ArrayPtr<byte> buffer, size_t minBytes);
  virtual size_t tryRead(ArrayPtr<byte> buffer, size_t minBytes) = 0;
  inline void read(ArrayPtr<byte> buffer) { read(buffer, buffer.size()); }

and new write API supports multiple pieces from the get go:

  virtual void write(ArrayPtr<const byte> data, 
                     ArrayPtr<const ArrayPtr<const byte>> moreData = nullptr) = 0;

@mikea mikea force-pushed the maizatskyi/2024-04-24-new-sync-io branch 12 times, most recently from 5920e22 to 3568a24 Compare May 3, 2024 16:34
@mikea mikea marked this pull request as ready for review May 3, 2024 16:35
@mikea
Copy link
Collaborator Author

mikea commented May 3, 2024

I think this is ready for review. I'll start working on downstream PRs meanwhile.

@mikea mikea force-pushed the maizatskyi/2024-04-24-new-sync-io branch from 3568a24 to ab0ce88 Compare May 3, 2024 16:46
c++/src/kj/io.h Outdated Show resolved Hide resolved
c++/src/kj/io.h Outdated Show resolved Hide resolved
@mikea mikea force-pushed the maizatskyi/2024-04-24-new-sync-io branch 4 times, most recently from 6924142 to d5f5953 Compare May 8, 2024 17:17
@kentonv
Copy link
Member

kentonv commented May 8, 2024

Looks like the MSVC tests are failing. (#2024 fixed them at master, so presumably it's a new breakage.)

Otherwise looks good.

@mikea mikea force-pushed the maizatskyi/2024-04-24-new-sync-io branch 4 times, most recently from c201f68 to e66c7e7 Compare May 9, 2024 17:33
@mikea mikea force-pushed the maizatskyi/2024-04-24-new-sync-io branch from e66c7e7 to 6b13999 Compare May 9, 2024 17:37
@jasnell
Copy link
Collaborator

jasnell commented May 9, 2024

LGTM once CI is green. I'll leave it to someone else to actually hit approve tho.

@mikea
Copy link
Collaborator Author

mikea commented May 9, 2024

Looks like the MSVC tests are failing. (#2024 fixed them at master, so presumably it's a new breakage.)

Windows build is fixed. I've opened downstream PRs that I will update now to the latest hash:

@mikea mikea force-pushed the maizatskyi/2024-04-24-new-sync-io branch from 6b13999 to 89b1d43 Compare May 14, 2024 15:41
@mikea
Copy link
Collaborator Author

mikea commented May 14, 2024

just a rebase

@mikea mikea merged commit 8653ebc into v2 May 16, 2024
13 checks passed
@mikea mikea deleted the maizatskyi/2024-04-24-new-sync-io branch May 16, 2024 20:16
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

3 participants