Support writing/reading bytes in librt.internal #20069
Conversation
This comment has been minimized.
This comment has been minimized.
| Py_ssize_t size; | ||
| _CHECK_READ(data, 1, NULL) | ||
| uint8_t first = _READ(data, uint8_t) | ||
| if (likely(first != LONG_STR_TAG)) { |
There was a problem hiding this comment.
Once we have 2-byte etc. integer formats, it could make sense to use the same integer format here, even if it's slightly non-ideal, so that we can share code.
There was a problem hiding this comment.
Yeah, as I mentioned in other issue I will try to simply code when adding tags for primitive types.
| def write_str(data: Buffer, value: str) -> None: ... | ||
| def read_str(data: Buffer) -> str: ... | ||
| def write_bytes(data: Buffer, value: bytes) -> None: ... | ||
| def read_bytes(data: Buffer) -> bytes: ... |
There was a problem hiding this comment.
Somewhat meta: what if we'd make all these accept positional arguments only? We could then generate slightly faster wrapper methods for interpreted use cases perhaps (this would affect other methods as well, so the change would be in a separate PR).
There was a problem hiding this comment.
FWIW I am not worried about interpreted performance that much, current code is already relatively efficient, but I am not against if someone else will do this.
|
According to mypy_primer, this change doesn't affect type check results on a corpus of open source code. ✅ |
|
@JukkaL so do you want to release this one yourself? The steps are:
If you don't have time, I can do it myself. But from my experience it is very fast, whole process takes ~15min |
|
Okay, I'll try to follow the steps (today or tomorrow probably). |
|
I'm not sure what's the right way build and install the library and run tests locally. I think this should be documented as well -- I tried a few obvious things but they didn't work. |
|
Oh I think |
|
To give some context on why it is made that way, it is because I wanted the part that is synced to be located in a directory, to not clutter the top level, but since this includes Btw don't worry if it will say |
|
Now the 0.3.0 release of librt is up, with these changes. I'll create a PR to add more detailed release instructions to the librt repo. |
See discussion at python/mypy#20069 for additional context.
2 participants
This is for storing actual
bytesobjects (not arbitrary sub-buffers). This will be useful for storing various hashes in cache metas, since storing them as hex strings takes twice more space and currently these hashes take a macroscopic part of the cache metas' sizes.@JukkaL for training purposes you can sync/release this one yourself (if you want to, of course).