Repository navigation
Open
Description
Is it possible to avoid memory copies when writing to S3 by taking advantage of the Python buffer protocol?
In particular, it would be great if we could use a memoryview object in calls to pipe().
Using bytes and bytearray work fine:
>>> import numpy as np
>>> import fsspec
>>> fs = fsspec.filesystem('s3')
>>> fs.pipe("s3://cubed-unittest/mem-array/s3test", b"hello") # works
[{'ResponseMetadata': {'RequestId': 'E54C5V2H9M1ZE1V1', 'HostId': 'pcEW+k8KZHBbYuohqfgGNZOac8Y7QzRAHaAxOJmMWQdEll75umxh+On2DXxWi8qKMcG2+aZmJ7I=', 'HTTPStatusCode': 200, 'HTTPHeaders': {'x-amz-id-2': 'pcEW+k8KZHBbYuohqfgGNZOac8Y7QzRAHaAxOJmMWQdEll75umxh+On2DXxWi8qKMcG2+aZmJ7I=', 'x-amz-request-id': 'E54C5V2H9M1ZE1V1', 'date': 'Thu, 10 Apr 2025 13:19:42 GMT', 'x-amz-expiration': 'expiry-date="Sat, 12 Apr 2025 00:00:00 GMT", rule-id="gc"', 'x-amz-server-side-encryption': 'AES256', 'etag': '"5d41402abc4b2a76b9719d911017c592"', 'x-amz-checksum-crc32': 'NhCmhg==', 'x-amz-checksum-type': 'FULL_OBJECT', 'content-length': '0', 'server': 'AmazonS3'}, 'RetryAttempts': 0}, 'Expiration': 'expiry-date="Sat, 12 Apr 2025 00:00:00 GMT", rule-id="gc"', 'ETag': '"5d41402abc4b2a76b9719d911017c592"', 'ChecksumCRC32': 'NhCmhg==', 'ChecksumType': 'FULL_OBJECT', 'ServerSideEncryption': 'AES256'}]
>>> fs.pipe("s3://cubed-unittest/mem-array/s3test", bytearray(b"hello")) # works
[{'ResponseMetadata': {'RequestId': 'C58QYEAVJTBW96Y6', 'HostId': 'lwi6otZoaiTsPhLYApOdMmhFXphWEfhoYh/sRO4txe88uUnj8wLNHTgaleq7agiipYlyd8GHlzrxrZ8sAF1qQIuyXQUwm5Nq', 'HTTPStatusCode': 200, 'HTTPHeaders': {'x-amz-id-2': 'lwi6otZoaiTsPhLYApOdMmhFXphWEfhoYh/sRO4txe88uUnj8wLNHTgaleq7agiipYlyd8GHlzrxrZ8sAF1qQIuyXQUwm5Nq', 'x-amz-request-id': 'C58QYEAVJTBW96Y6', 'date': 'Thu, 10 Apr 2025 13:20:04 GMT', 'x-amz-expiration': 'expiry-date="Sat, 12 Apr 2025 00:00:00 GMT", rule-id="gc"', 'x-amz-server-side-encryption': 'AES256', 'etag': '"5d41402abc4b2a76b9719d911017c592"', 'x-amz-checksum-crc32': 'NhCmhg==', 'x-amz-checksum-type': 'FULL_OBJECT', 'content-length': '0', 'server': 'AmazonS3'}, 'RetryAttempts': 0}, 'Expiration': 'expiry-date="Sat, 12 Apr 2025 00:00:00 GMT", rule-id="gc"', 'ETag': '"5d41402abc4b2a76b9719d911017c592"', 'ChecksumCRC32': 'NhCmhg==', 'ChecksumType': 'FULL_OBJECT', 'ServerSideEncryption': 'AES256'}]But using memoryview or just a numpy array (which supports the buffer protocol too) fail:
>>> fs.pipe("s3://cubed-unittest/mem-array/s3test", memoryview(np.frombuffer(bytearray(b"hello"), dtype="uint8")))
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
File "/Users/tom/miniforge3/envs/memray-array/lib/python3.11/site-packages/fsspec/asyn.py", line 118, in wrapper
return sync(self.loop, func, *args, **kwargs)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/Users/tom/miniforge3/envs/memray-array/lib/python3.11/site-packages/fsspec/asyn.py", line 103, in sync
raise return_result
File "/Users/tom/miniforge3/envs/memray-array/lib/python3.11/site-packages/fsspec/asyn.py", line 56, in _runner
result[0] = await coro
^^^^^^^^^^
File "/Users/tom/miniforge3/envs/memray-array/lib/python3.11/site-packages/fsspec/asyn.py", line 418, in _pipe
return await _run_coros_in_chunks(
^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/Users/tom/miniforge3/envs/memray-array/lib/python3.11/site-packages/fsspec/asyn.py", line 268, in _run_coros_in_chunks
result, k = await done.pop()
^^^^^^^^^^^^^^^^
File "/Users/tom/miniforge3/envs/memray-array/lib/python3.11/site-packages/fsspec/asyn.py", line 245, in _run_coro
return await asyncio.wait_for(coro, timeout=timeout), i
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/Users/tom/miniforge3/envs/memray-array/lib/python3.11/asyncio/tasks.py", line 452, in wait_for
return await fut
^^^^^^^^^
File "/Users/tom/miniforge3/envs/memray-array/lib/python3.11/site-packages/s3fs/core.py", line 1185, in _pipe_file
out = await self._call_s3(
^^^^^^^^^^^^^^^^^^^^
File "/Users/tom/miniforge3/envs/memray-array/lib/python3.11/site-packages/s3fs/core.py", line 371, in _call_s3
return await _error_wrapper(
^^^^^^^^^^^^^^^^^^^^^
File "/Users/tom/miniforge3/envs/memray-array/lib/python3.11/site-packages/s3fs/core.py", line 146, in _error_wrapper
raise err
File "/Users/tom/miniforge3/envs/memray-array/lib/python3.11/site-packages/s3fs/core.py", line 114, in _error_wrapper
return await func(*args, **kwargs)
^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/Users/tom/miniforge3/envs/memray-array/lib/python3.11/site-packages/aiobotocore/client.py", line 369, in _make_api_call
request_dict = await self._convert_to_request_dict(
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/Users/tom/miniforge3/envs/memray-array/lib/python3.11/site-packages/aiobotocore/client.py", line 440, in _convert_to_request_dict
request_dict = self._serializer.serialize_to_request(
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/Users/tom/miniforge3/envs/memray-array/lib/python3.11/site-packages/botocore/validate.py", line 381, in serialize_to_request
raise ParamValidationError(report=report.generate_report())
botocore.exceptions.ParamValidationError: Parameter validation failed:
Invalid type for parameter Body, value: <memory at 0x102f99a80>, type: <class 'memoryview'>, valid types: <class 'bytes'>, <class 'bytearray'>, file-like objectThis may be a limitation in the underlying boto library, but perhaps there is a way to avoid copying the source data (in this case a numpy array) that I'm missing.
cc @jakirkham who commented about this in zarr-developers/zarr-python#2972 (comment)
Reacted by Josh Moore
Metadata
Metadata
Assignees
Labels
No labels
Activity
jakirkham commentedon Apr 12, 2025
Thanks Tom! 🙏
It seems like the error is coming from
botocoreThe traceback cuts short of the indicating the specific function, but reading the tea leaves leads me to conclude this
botocoreinput validation function is involvedNow fixing that could resolve the issue. OTOH there may be other assumptions made in the code that are less clear. That said, there usually is a path forward with
memoryviews (usually with some tweaks)Looking around found a similar upstream issue: boto/boto3#3423
There is some work to fix that in PR: boto/botocore#3107
Unfortunately there has not been a lot of engagement as of yet. Tried to do a rough review of the PR to see if that might help move things along
tomwhite commentedon Apr 14, 2025
Thanks for digging into this @jakirkham!
I tried rebasing the botocore PR on main then running the fsspec test in this issue again. I got:
This is because the body is a
aiobotocore.httpchecksum.AioAwsChunkedWrapper, not just bytes. So I worked around that, then got:So it looks like
memoryviewis not sufficient.BTW I also stumbled across this article: https://pythonspeed.com/articles/bytesio-reduce-memory-usage/, with the intriguing suggestion that using
BytesIOcan avoid memory copies (if you start with aBytesIOobject, at least). But afaict there's no way to go from a numpy array toBytesIOwithout a copy, which is what we need in the Zarr case that started this discussion...martindurant commentedon Apr 14, 2025
I have a couple of suggestions to try:
which might just "do the right thing", depending on what botocore actually does with it.
This is enough to pass the isinstance check.
tomwhite commentedon Apr 15, 2025
Thank you for the suggestions @martindurant!
I tried using cramjam's
Bufferto wrap a NumPy array, but it seems to copy the bytes on construction. This also happens when using a NumPy array wrapped in amemoryview.So that leaves
ErsatzByteswhich may be possible, but needs more investigation.martindurant commentedon Apr 15, 2025
@milesgranger - is there a correct invocation to use one of the cramjam buffer types to pass numpy/memoryview data to a third party with zero copy? That third party officially accepts bytes, bytearray and BytesIO.
milesgranger commentedon Apr 15, 2025
Cramjam's (only) Buffer type owns its data https://github.com/milesgranger/cramjam/blob/ebc9ce798c8f86871a32cb8bf9e515e45468f77c/src/io.rs#L342, but of course implements buffer protocol should one want to pass its data to a third-party with zero copy.
Anyhow, interesting and a very plausible implementation detail to support this behavior, but not directly applicable for cramjam as all the compression bits (which is cramjam's...jam.) implements zero-copy input and optionally zero-copy output w/ its
_intofunctions, so not directly relevant forcramjam.Bufferto implement a zero copy overlay to numpy arrays or anything else for that matter.martindurant commentedon Apr 15, 2025
Yeah, it seems like the pyo3 ecosystem is now at the point where it can provide some nice behind-the-scenes benefits like zero-copy where otherwise it wouldn't work. Not sure, then, how to actually make that happen!
I believe the rationale for having bytes (and str) own its memory and not be mutable, is that they han then be hashed in dict/sets. Many applications don't care about that, though.
milesgranger commentedon Apr 15, 2025
I agree, and partly why I'm quite unmotivated to remove cramjam's current ability to overwrite
bytesas an output destination... I should clarify, I'm definitely open to adding this functionality! Seems most of the pieces are already there and if even a few people would find it helpful, I think it's relatively low overhead to add it.martindurant commentedon Apr 15, 2025
Well that's great! It still might not solve the original problem, though, depending on exactly what isinstance or similar checks are done within botocore. Still, once such a thing is possible, people ought to start using it.
kylebarron commentedon Apr 21, 2025
FWIW the
obstore.Bytestype allows externally-allocated memory via the buffer protocol (and viapyo3-bytes) and is slowly progressing to support all the methods onbytes.We could probably mix and match that and the
RustyBuffermethods to get you what you want.martindurant commentedon Apr 21, 2025
Good to hear, @kylebarron . Does it inherit from Bytes too?
kylebarron commentedon Apr 21, 2025
You mean subclass? That's not possible because it doesn't use
builtins.bytesinternally. But it can match thebytesmethods as a protocol as closely as possible.martindurant commentedon Apr 21, 2025
For the purposes of the use here, it might be useful to have a pretend subclass
(obstore.Bytes, builtins.bytes)for the purposes of third-party libraries that useissubclassto make decisions.kylebarron commentedon Apr 21, 2025
I think that's a valid choice for downstream users but not something that
obstore.Bytesshould implement itself.milesgranger commentedon Apr 24, 2025
@tomwhite, although I'm sure some in the community may frown upon it, I have no such scruples and found it interesting enough to take a stab at it.
pip install cramjam==2.11.0rc1-cramjam.Buffer(..., copy=False)v2.11.0-rc1
Open any related issues over there if you please.
tomwhite commentedon Apr 24, 2025
Thank you for working on this @milesgranger!
I used
copy=Falseand it now avoids a copy (verified using memray). But using it in s3fs still has problems, and unfortunately the stacktrace is not very helpful:martindurant commentedon Apr 24, 2025
@tomwhite : it makes it look like a network correction fault; just checking that writing a
bytesdoes still work?If yes, it would be helpful to somehow log what calls botocore is making on the Bytes object.
tomwhite commentedon Apr 24, 2025
Yes, that still works.
+1
milesgranger commentedon Apr 24, 2025
I tried quick on a Minio deployment I had handy, slightly more informative but not by much, "ExcessData" error I'd never seen before. But the zero-copy buffer works fine with
filesystem('file'); not sure what needs to be added/changed to make boto happy, don't have time at the moment to poke any deeper.Note, that this also fails with io.BytesIO and the full-copy version of cramjam.Buffer
Traceback: `OSError: [Errno 5] An error occurred (ExcessData) when calling the PutObject operation: More data provided than indicated content length`
tomwhite commentedon Apr 24, 2025
Not sure if this is related:
Shouldn't it read to EOF and not have the trailing null bytes?
milesgranger commentedon Apr 24, 2025
Oof, ya. But since fs.pipe fails with copy=True (default) I sorta doubt that's part of this problem.
martindurant commentedon Apr 24, 2025
Is that
bytesgiven byread()actually a view, then?milesgranger commentedon Apr 24, 2025
No, it returns
bytesfrom its view.kylebarron commentedon Apr 24, 2025
Also fwiw
cramjam.Bufferadds support for a writable file-like BytesIO object in Rust-allocated memory, but I just realized that if you only need a readable file-like BytesIO object, you can useBytesIOdirectly, since it supports buffer protocol input:but you shouldn't mutate the underlying rust buffer, because
obstore.Byteshas invariants that assume the buffer won't be changing.milesgranger commentedon Apr 25, 2025
Figured it out, I have
fsspec.filesystem('s3').pipe(...)w/ a zero-copy cramjam Buffer working, can install version2.11.0rc2For the
read() -> bytesnot being a subset view of the data, that was intentional as it's obviously a bit safer but namely things like aiobotocore reads in chunks when processing larger data so I assumed the small incremental reads would be fine.I might be convinced to add a
read(..., as_view=True)flag though; but I don't think that's what the original intention of this issue was - only to pass a "file-like-buffer-protocol-compatible-zero-copy-object" to pipe.tomwhite commentedon Apr 25, 2025
Can confirm that it works now. Thanks @milesgranger!
martindurant commentedon Apr 25, 2025
This is probably true. If it immediately does read() to get the whole thing as bytes, that would be annoying. It is supposed to support streaming in the sense you mean.
milesgranger commentedon Apr 25, 2025
It is true: