Add VmModule.mmap() to Python API. #14124
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
We really need page aligned flatbuffer blobs vs normal malloc alignment. The best way to be loading a file is via mmap, so just make that available as an API. This could be done in Python by the caller but is error-prone. The public API will make this more robust. Provides the mechanism to fix #13887
powderluv
approved these changes
Jun 15, 2023
|
May need some tweaking for Windows |
Abhishek-Varma
added a commit
to Abhishek-Varma/SHARK
that referenced
this pull request
Jun 15, 2023
-- This commit is based on [VmModule.mmap() API](iree-org/iree#14124). -- It thereby adds capability to mmap vmfbs in SHARK. Signed-off-by: Abhishek Varma <[email protected]>
ScottTodd
reviewed
Jun 15, 2023
Comment on lines
+119
to
+122
| with tempfile.NamedTemporaryFile() as tf: | ||
| tf.write(binary) | ||
| tf.flush() | ||
| m = iree.runtime.VmModule.mmap(self.instance, tf.name) |
There was a problem hiding this comment.
May need some tweaking for Windows
Indeed. https://github.com/openxla/iree/actions/runs/5275962562/jobs/9542060597#step:9:2321
ERROR: test_mmap (__main__.VmTest)
----------------------------------------------------------------------
Traceback (most recent call last):
File "C:\a\iree\iree\runtime\bindings\python\tests\vm_test.py", line 122, in test_mmap
m = iree.runtime.VmModule.mmap(self.instance, tf.name)
PermissionError: [Errno 13] Permission denied: 'C:\\Users\\RUNNER~1\\AppData\\Local\\Temp\\tmpr4u5ldjo'
Maybe related to #13148 (comment)
See https://stackoverflow.com/a/23212515 - NamedTemporaryFile creates and opens the file, and the file cannot be opened again... on Windows (it can be opened again on Unix).
There was a problem hiding this comment.
I've added a fix for that in SHARK to deal with this very issue - perhaps that can be used.
But the other issue in Windows which I bumped into is mentioned here - it's API related instead of the use case.
CC: @powderluv
Abhishek-Varma
added a commit
to Abhishek-Varma/SHARK
that referenced
this pull request
Jun 15, 2023
-- This commit is based on [VmModule.mmap() API](iree-org/iree#14124). -- It thereby adds capability to mmap vmfbs in SHARK. Signed-off-by: Abhishek Varma <[email protected]>
Abhishek-Varma
added a commit
to Abhishek-Varma/SHARK
that referenced
this pull request
Jun 15, 2023
-- This commit is based on [VmModule.mmap() API](iree-org/iree#14124). -- It thereby adds capability to mmap vmfbs in SHARK. Signed-off-by: Abhishek Varma <[email protected]>
stellaraccident
added a commit
that referenced
this pull request
Jun 15, 2023
Python mmap on Windows has a slightly different signature. Reverting for now to unbreak the Windows CI. Will re-apply when have access to test it there. This reverts commit 3345b76.
Abhishek-Varma
added a commit
to Abhishek-Varma/SHARK
that referenced
this pull request
Jun 21, 2023
-- This commit is based on [VmModule.mmap() API](iree-org/iree#14124). -- It thereby adds capability to mmap vmfbs in SHARK. Signed-off-by: Abhishek Varma <[email protected]>
Abhishek-Varma
added a commit
to Abhishek-Varma/SHARK
that referenced
this pull request
Jun 21, 2023
-- This commit is based on [VmModule.mmap() API](iree-org/iree#14124). -- It thereby adds capability to mmap vmfbs in SHARK. Signed-off-by: Abhishek Varma <[email protected]>
Abhishek-Varma
added a commit
to Abhishek-Varma/SHARK
that referenced
this pull request
Jun 22, 2023
-- This commit is based on [VmModule.mmap() API](iree-org/iree#14124). -- It thereby adds capability to mmap vmfbs in SHARK. Signed-off-by: Abhishek Varma <[email protected]>
Abhishek-Varma
added a commit
to nod-ai/SHARK-Studio
that referenced
this pull request
Jun 22, 2023
-- This commit is based on [VmModule.mmap() API](iree-org/iree#14124). -- It thereby adds capability to mmap vmfbs in SHARK. Signed-off-by: Abhishek Varma <[email protected]>
nhasabni
pushed a commit
to plaidml/iree
that referenced
this pull request
Aug 24, 2023
We really need page aligned flatbuffer blobs vs normal malloc alignment. The best way to be loading a file is via mmap, so just make that available as an API. This could be done in Python by the caller but is error-prone. The public API will make this more robust. Provides the mechanism to fix iree-org#13887
nhasabni
pushed a commit
to plaidml/iree
that referenced
this pull request
Aug 24, 2023
Python mmap on Windows has a slightly different signature. Reverting for now to unbreak the Windows CI. Will re-apply when have access to test it there. This reverts commit 3345b76.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
We really need page aligned flatbuffer blobs vs normal malloc alignment. The best way to be loading a file is via mmap, so just make that available as an API.
This could be done in Python by the caller but is error-prone. The public API will make this more robust.
Provides the mechanism to fix #13887