Handle MemoryError for sequences#5418
Conversation
crazymerlyn
force-pushed
the
memoryerror
branch
from
ff4df62 to
9ad4d8f
Compare
vm/src/vm/mod.rs
Outdated
| pub(crate) use method::PyMethod; | ||
| pub use setting::Settings; | ||
|
|
||
| pub const MAX_MEMORY_SIZE: usize = (usize::MAX >> 3) + 1; |
There was a problem hiding this comment.
Thank you! Is this constant came from CPython?
There was a problem hiding this comment.
Nah, I just copied it from the last PR. CPython seems to use max usize instead: https://github.com/python/cpython/blob/e0a41a5dd12cb6e9277b05abebac5c70be684dd7/Objects/listobject.c#L740, https://github.com/python/cpython/blob/e0a41a5dd12cb6e9277b05abebac5c70be684dd7/Include/pyport.h#L146
There was a problem hiding this comment.
That looks like isize::MAX, since the first s in ssize_t means signed. Which makes sense, since that's the max size an allocation can be for LLVM. I do think we could maybe just use isize::MAX, because otherwise we're unnecessarily constraining allocation size to "just" 512GiB on 32-bit systems.
There was a problem hiding this comment.
Updated to use isize::MAX. Also updated to include the size of the type and fixed the potential integer overflow problem.
|
Added @coolreader18 as a reviewer who reviewed #1779 |
crazymerlyn
force-pushed
the
memoryerror
branch
2 times, most recently
from
75a03e3 to
cf9f036
Compare
crazymerlyn
force-pushed
the
memoryerror
branch
from
cf9f036 to
6fb19ac
Compare
coolreader18
left a comment
coolreader18
left a comment
There was a problem hiding this comment.
LGTM, thanks for contributing!
3 participants
Fixes #1750
Based on #1779 which seems to be abandoned.