From 88a186eae8b98e357603aff3bc46fda508a3c21a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Lozier?= Date: Sun, 27 Sep 2026 12:47:01 -0400 Subject: [PATCH 1/2] Improve struct.unpack_from performance with mmap --- src/core/IronPython.Modules/_struct.cs | 80 ++++++++++++++++--- .../Runtime/Binding/PythonOverloadResolver.cs | 14 +++- 2 files changed, 79 insertions(+), 15 deletions(-) diff --git a/src/core/IronPython.Modules/_struct.cs b/src/core/IronPython.Modules/_struct.cs index cdebb289c..2bc6260fc 100644 --- a/src/core/IronPython.Modules/_struct.cs +++ b/src/core/IronPython.Modules/_struct.cs @@ -425,12 +425,27 @@ public void pack_into(CodeContext/*!*/ context, [NotNone] IBufferProtocol/*!*/ b [Documentation("reads the current format from the specified array")] public PythonTuple/*!*/ unpack_from(CodeContext/*!*/ context, [BytesLike][NotNone] IList/*!*/ buffer, int offset = 0) { - int bytesAvail = buffer.Count - offset; - if (bytesAvail < size) { + offset = NormalizeOffset(context, offset, buffer.Count); + return unpack(context, buffer.Substring(offset, size)); + } + + [Documentation("reads the current format from the specified array")] + public PythonTuple/*!*/ unpack_from(CodeContext/*!*/ context, [NotNone] IBufferProtocol/*!*/ buffer, int offset = 0) { + using var buf = buffer.GetBuffer(BufferFlags.Simple); + var span = buf.AsReadOnlySpan(); + offset = NormalizeOffset(context, offset, span.Length); + return unpack(context, span.Slice(offset, size).ToArray()); + } + + private int NormalizeOffset(CodeContext context, int offset, int length) { + if (offset < 0) { + offset += length; + } + int bytesAvail = length - offset; + if (offset < 0 || bytesAvail < size) { throw Error(context, $"unpack_from requires a buffer of at least {size} bytes"); } - - return unpack(context, buffer.Substring(offset, size)); + return offset; } [Documentation("iteratively unpack the current format from the specified array.")] @@ -438,6 +453,11 @@ public PythonUnpackIterator iter_unpack(CodeContext/*!*/ context, [BytesLike][No return new PythonUnpackIterator(this, context, buffer); } + [Documentation("iteratively unpack the current format from the specified array.")] + public PythonUnpackIterator iter_unpack(CodeContext/*!*/ context, [NotNone] IBufferProtocol/*!*/ buffer) { + return new PythonUnpackIterator(this, context, buffer.GetBuffer(BufferFlags.Simple)); + } + [Documentation("gets the number of bytes that the serialized string will occupy or are required to deserialize the data")] public int size { get { @@ -676,17 +696,21 @@ internal static Struct Create(string/*!*/ format) { #endregion } +#nullable enable + [PythonType("unpack_iterator"), Documentation("Represents an iterator returned by _struct.iter_unpack()")] public sealed class PythonUnpackIterator : IEnumerator, IEnumerable { - private object _iter_current; + private object? _iter_current; private int _next_offset; private readonly CodeContext _context; - private readonly IList _buffer; + private readonly IList? _buffer; + private readonly IPythonBuffer? _pythonBuffer; private readonly Struct _owner; internal PythonUnpackIterator(Struct/*!*/ owner, CodeContext/*!*/ context, IList/*!*/ buffer) { _context = context; + _pythonBuffer = null; _buffer = buffer; _owner = owner; @@ -695,11 +719,24 @@ internal PythonUnpackIterator(Struct/*!*/ owner, CodeContext/*!*/ context, IList ValidateBufferLength(); } + internal PythonUnpackIterator(Struct/*!*/ owner, CodeContext/*!*/ context, IPythonBuffer/*!*/ buffer) { + _context = context; + _pythonBuffer = buffer; + _buffer = null; + _owner = owner; + + _iter_current = null; + _next_offset = 0; + ValidateBufferLength(); + } + + private int BufferLength => _pythonBuffer?.NumBytes() ?? _buffer!.Count; + private void ValidateBufferLength() { if (_owner.size == 0) { throw Error(_context, "cannot iteratively unpack with a struct of length 0"); } - if (_buffer.Count % _owner.size != 0) { + if (BufferLength % _owner.size != 0) { throw Error(_context, $"iterative unpacking requires a buffer of a multiple of {_owner.size} bytes"); } } @@ -716,15 +753,20 @@ private void ValidateBufferLength() { #region IEnumerator Members [PythonHidden] - public object Current => _iter_current; + public object Current => _iter_current!; [PythonHidden] public bool MoveNext() { - if (_buffer.Count - _next_offset < _owner.size) { + if (BufferLength - _next_offset < _owner.size) { return false; } - _iter_current = _owner.unpack_from(_context, _buffer, _next_offset); + if (_pythonBuffer is null) { + _iter_current = _owner.unpack_from(_context, _buffer, _next_offset); + } + else { + _iter_current = _owner.unpack(_context, _pythonBuffer.AsReadOnlySpan().Slice(_next_offset, _owner.size).ToArray()); + } _next_offset += _owner.size; return true; } @@ -732,14 +774,18 @@ public bool MoveNext() { void IEnumerator.Reset() => throw new NotSupportedException(); [PythonHidden] - public void Dispose() { } + public void Dispose() { + _pythonBuffer?.Dispose(); + } #endregion public int __length_hint__() - => (_buffer.Count - _next_offset) / _owner.size; + => (BufferLength - _next_offset) / _owner.size; } +#nullable restore + #endregion #region Compiled Format @@ -894,11 +940,21 @@ public static void pack_into(CodeContext/*!*/ context, object fmt, [NotNone] IBu return GetStructFromCache(context, fmt).unpack_from(context, buffer, offset); } + [Documentation("Unpack the buffer, containing packed C structure data, according to\nfmt, starting at offset. Requires len(buffer[offset:]) >= calcsize(fmt).")] + public static PythonTuple/*!*/ unpack_from(CodeContext/*!*/ context, object fmt, [NotNone] IBufferProtocol/*!*/ buffer, int offset = 0) { + return GetStructFromCache(context, fmt).unpack_from(context, buffer, offset); + } + [Documentation("Iteratively unpack the buffer, containing packed C structure data, according to\nfmt, starting at offset. Requires len(buffer[offset:]) >= calcsize(fmt).")] public static PythonUnpackIterator/*!*/ iter_unpack(CodeContext/*!*/ context, object fmt, [BytesLike][NotNone] IList/*!*/ buffer) { return GetStructFromCache(context, fmt).iter_unpack(context, buffer); } + [Documentation("Iteratively unpack the buffer, containing packed C structure data, according to\nfmt, starting at offset. Requires len(buffer[offset:]) >= calcsize(fmt).")] + public static PythonUnpackIterator/*!*/ iter_unpack(CodeContext/*!*/ context, object fmt, [NotNone] IBufferProtocol/*!*/ buffer) { + return GetStructFromCache(context, fmt).iter_unpack(context, buffer); + } + #endregion #region Write Helpers diff --git a/src/core/IronPython/Runtime/Binding/PythonOverloadResolver.cs b/src/core/IronPython/Runtime/Binding/PythonOverloadResolver.cs index 434d46f79..41912ffef 100644 --- a/src/core/IronPython/Runtime/Binding/PythonOverloadResolver.cs +++ b/src/core/IronPython/Runtime/Binding/PythonOverloadResolver.cs @@ -72,6 +72,14 @@ public override Candidate SelectBestConversionFor(DynamicMetaObject arg, Paramet return basePreferred; } + // Prefer the [BytesLike] overload when the argument type is assignable to it. + if (IsBytesLikeParameter(candidateTwo)) { + return candidateTwo.Type.IsAssignableFrom(arg.LimitType) ? Candidate.Two : Candidate.One; + } + if (IsBytesLikeParameter(candidateOne)) { + return candidateOne.Type.IsAssignableFrom(arg.LimitType) ? Candidate.One : Candidate.Two; + } + // Work around the choice made in Converter.PreferConvert // This cannot be done using NarrowingLevel rules because it would confuse rules for selecting custom operators if (level >= PythonNarrowing.IndexOperator && Converter.IsPythonBigInt(arg.LimitType)) { @@ -141,15 +149,15 @@ public override bool CanConvertFrom(Type fromType, DynamicMetaObject fromArg, Pa Type toType = toParameter.Type; if (IsBytesLikeParameter(toParameter)) { - - if ((fromType == typeof(PythonList) || fromType.IsSubclassOf(typeof(PythonList)))) { + if (fromType == typeof(PythonList) || fromType.IsSubclassOf(typeof(PythonList))) { if (toType.IsGenericType && toType.GetGenericTypeDefinition() == typeof(IList<>)) { return false; } } - if (typeof(IBufferProtocol).IsAssignableFrom(fromType)) { + // Apply this conversion only to real arguments, not parameter-type comparisons (in which case fromArg is null). + if (fromArg is not null && typeof(IBufferProtocol).IsAssignableFrom(fromType)) { if (toParameter.Type == typeof(IList) || toParameter.Type == typeof(IReadOnlyList)) { return true; } From d1083f49a1a810147f4c54ac41b6fd05a157d316 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Lozier?= Date: Sun, 27 Sep 2026 12:57:24 -0400 Subject: [PATCH 2/2] Add test to cover unpack_from negative offset --- tests/suite/test_struct.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/tests/suite/test_struct.py b/tests/suite/test_struct.py index 819838f70..890b1c679 100644 --- a/tests/suite/test_struct.py +++ b/tests/suite/test_struct.py @@ -42,6 +42,13 @@ def test_unpack_from(self): a, = struct.unpack_from(b'>H', b"\x00\x01") self.assertEqual(a, 1) + data = b"\x00\x01\x00\x02" + a, = struct.unpack_from('>H', data, -2) + self.assertEqual(a, 2) + + a, = struct.unpack_from('>H', memoryview(data), -2) + self.assertEqual(a, 2) + def test_pack_into(self): # test string format string result = array.array('b', [0, 0])