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

FoundationEssentials: support mmap'ed reading on Windows #527

Merged
merged 1 commit into from May 6, 2024

Conversation

compnerd
Copy link
Collaborator

@compnerd compnerd commented Apr 3, 2024

This adjusts the data reading path for supporting Windows which does not have the mmap function but does provide memory-mapped IO. The difference in the API is sufficient to create a separate path for the wrapping function rather than trying to have a single shared implementation that simply uses different APIs for the system integration.

@compnerd compnerd requested a review from parkera April 3, 2024 22:24
@compnerd
Copy link
Collaborator Author

compnerd commented Apr 3, 2024

Note that this doesn't currently work as I don't know how to handle the errors. The error information is not really an errno (POSIX error code) but rather a Win32 Error (GetLastError). The build failure on Windows for this file is now just the error reporting.

@jmschonfeld
Copy link
Contributor

Note that this doesn't currently work as I don't know how to handle the errors

CocoaError.errorWithFilePath with an errno just creates a CocoaError by mapping the errno to an applicable CocoaError.Code and then creates a CocoaError with that code and a user info dictionary with the underlying POSIXError. Would we be able to create another overload of errorWithFilePath but instead of an errno: parameter perhaps with a win32Error: parameter that similarly maps error codes to CocoaError.Code and then creates a user info dictionary with just the underlying code instead of using a POSIXError?

@compnerd
Copy link
Collaborator Author

compnerd commented Apr 4, 2024

Thanks @jmschonfeld that sounds like it should be possible to do!

@compnerd compnerd force-pushed the mmap branch 2 times, most recently from 387c084 to 3c74198 Compare April 4, 2024 18:07
@compnerd
Copy link
Collaborator Author

compnerd commented Apr 4, 2024

I've created #528 with the new error mapping that can hopefully be used to build this out.

@compnerd
Copy link
Collaborator Author

compnerd commented Apr 5, 2024

@swift-ci please test

This adjusts the data reading path for supporting Windows which does not
have the `mmap` function but does provide memory-mapped IO. The
difference in the API is sufficient to create a separate path for the
wrapping function rather than trying to have a single shared
implementation that simply uses different APIs for the system
integration.
@compnerd
Copy link
Collaborator Author

compnerd commented May 2, 2024

@swift-ci please test

@jmschonfeld jmschonfeld merged commit a76eaee into apple:main May 6, 2024
2 checks passed
@compnerd compnerd deleted the mmap branch May 7, 2024 19:12
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

2 participants