From b0a5fe827fcd249105be296436723a123d82b31c Mon Sep 17 00:00:00 2001 From: Jason Nelson Date: Wed, 21 Feb 2024 22:40:12 -0800 Subject: [PATCH 1/8] Add ScopedBuffer --- MetadataExtractor/IO/ScopedBuffer.cs | 43 ++++++++++++++++++++++++++++ 1 file changed, 43 insertions(+) create mode 100644 MetadataExtractor/IO/ScopedBuffer.cs diff --git a/MetadataExtractor/IO/ScopedBuffer.cs b/MetadataExtractor/IO/ScopedBuffer.cs new file mode 100644 index 00000000..eddae690 --- /dev/null +++ b/MetadataExtractor/IO/ScopedBuffer.cs @@ -0,0 +1,43 @@ +// Copyright (c) Drew Noakes and contributors. All Rights Reserved. Licensed under the Apache License, Version 2.0. See LICENSE in the project root for license information. + +using System.Buffers; + +namespace MetadataExtractor.IO; + +internal ref struct ScopedBuffer +{ + private byte[]? _rentedBuffer; + private readonly Span _span; + + public ScopedBuffer(int size) + { + _rentedBuffer = ArrayPool.Shared.Rent(size); + _span = _rentedBuffer.AsSpan(0, size); + } + + public ScopedBuffer(Span span) + { + _span = span; + } + + public readonly Span Span => _span; + + public static implicit operator Span(ScopedBuffer bufferScope) + { + return bufferScope._span; + } + + public static implicit operator ReadOnlySpan(ScopedBuffer bufferScope) + { + return bufferScope._span; + } + + public void Dispose() + { + if (_rentedBuffer is null) return; + + ArrayPool.Shared.Return(_rentedBuffer); + + _rentedBuffer = null; + } +} From 215881dcfeb6ec5636f810985591bf393a9b3432 Mon Sep 17 00:00:00 2001 From: Jason Nelson Date: Wed, 21 Feb 2024 22:40:43 -0800 Subject: [PATCH 2/8] Use scoped buffer (part 1) --- MetadataExtractor/IO/BufferReader.Indexed.cs | 12 +++--------- MetadataExtractor/IO/IndexedReader.cs | 12 +++--------- MetadataExtractor/IO/SequentialReader.cs | 12 +++--------- 3 files changed, 9 insertions(+), 27 deletions(-) diff --git a/MetadataExtractor/IO/BufferReader.Indexed.cs b/MetadataExtractor/IO/BufferReader.Indexed.cs index 398644f8..e2f12c3f 100644 --- a/MetadataExtractor/IO/BufferReader.Indexed.cs +++ b/MetadataExtractor/IO/BufferReader.Indexed.cs @@ -193,17 +193,11 @@ public readonly string GetString(int index, int bytesRequested, Encoding encodin } else { - byte[] bytes = ArrayPool.Shared.Rent(bytesRequested); + using var buffer = new ScopedBuffer(bytesRequested); - Span span = bytes.AsSpan(0, bytesRequested); + GetBytes(index, buffer); - GetBytes(index, span); - - var s = encoding.GetString(span); - - ArrayPool.Shared.Return(bytes); - - return s; + return encoding.GetString(buffer); } } diff --git a/MetadataExtractor/IO/IndexedReader.cs b/MetadataExtractor/IO/IndexedReader.cs index 9cedc8b6..4126d833 100644 --- a/MetadataExtractor/IO/IndexedReader.cs +++ b/MetadataExtractor/IO/IndexedReader.cs @@ -335,17 +335,11 @@ public string GetString(int index, int bytesRequested, Encoding encoding) } else { - byte[] bytes = ArrayPool.Shared.Rent(bytesRequested); + using var buffer = new ScopedBuffer(bytesRequested); - Span span = bytes.AsSpan(0, bytesRequested); + GetBytes(index, buffer); - GetBytes(index, span); - - var s = encoding.GetString(span); - - ArrayPool.Shared.Return(bytes); - - return s; + return encoding.GetString(buffer); } } diff --git a/MetadataExtractor/IO/SequentialReader.cs b/MetadataExtractor/IO/SequentialReader.cs index 29b4ab5f..055bead9 100644 --- a/MetadataExtractor/IO/SequentialReader.cs +++ b/MetadataExtractor/IO/SequentialReader.cs @@ -236,17 +236,11 @@ public string GetString(int bytesRequested, Encoding encoding) } else { - byte[] bytes = ArrayPool.Shared.Rent(bytesRequested); + using var buffer = new ScopedBuffer(bytesRequested); - Span span = bytes.AsSpan(0, bytesRequested); + GetBytes(buffer); - GetBytes(span); - - var s = encoding.GetString(span); - - ArrayPool.Shared.Return(bytes); - - return s; + return encoding.GetString(buffer); } } From 8ad19dc8b20318e538dd78907a0d2a48ca23b65b Mon Sep 17 00:00:00 2001 From: Jason Nelson Date: Wed, 21 Feb 2024 22:41:09 -0800 Subject: [PATCH 3/8] Optimize BplistReader.HandleDict --- MetadataExtractor/Formats/Apple/BplistReader.cs | 11 ++--------- 1 file changed, 2 insertions(+), 9 deletions(-) diff --git a/MetadataExtractor/Formats/Apple/BplistReader.cs b/MetadataExtractor/Formats/Apple/BplistReader.cs index f5a2fe59..320ed773 100644 --- a/MetadataExtractor/Formats/Apple/BplistReader.cs +++ b/MetadataExtractor/Formats/Apple/BplistReader.cs @@ -1,7 +1,5 @@ // Copyright (c) Drew Noakes and contributors. All Rights Reserved. Licensed under the Apache License, Version 2.0. See LICENSE in the project root for license information. -using System.Buffers; - namespace MetadataExtractor.Formats.Apple; /// @@ -123,12 +121,9 @@ static object HandleInt(ref BufferReader reader, byte marker) static Dictionary HandleDict(ref BufferReader reader, byte count) { - var keyRefs = ArrayPool.Shared.Rent(count); + Span keyRefs = stackalloc byte[count]; // <= 255 - for (int j = 0; j < count; j++) - { - keyRefs[j] = reader.GetByte(); - } + reader.GetBytes(keyRefs); Dictionary map = []; @@ -137,8 +132,6 @@ static Dictionary HandleDict(ref BufferReader reader, byte count) map.Add(keyRefs[j], reader.GetByte()); } - ArrayPool.Shared.Return(keyRefs); - return map; } From e76716a29e90d2159a608caae6d32521b861e0a3 Mon Sep 17 00:00:00 2001 From: Jason Nelson Date: Wed, 21 Feb 2024 22:48:02 -0800 Subject: [PATCH 4/8] Validate GetString parameters --- MetadataExtractor/IO/BufferReader.Indexed.cs | 3 +++ MetadataExtractor/IO/BufferReader.Sequential.cs | 14 ++++++++------ MetadataExtractor/IO/IndexedReader.cs | 3 +++ MetadataExtractor/IO/SequentialReader.cs | 3 +++ 4 files changed, 17 insertions(+), 6 deletions(-) diff --git a/MetadataExtractor/IO/BufferReader.Indexed.cs b/MetadataExtractor/IO/BufferReader.Indexed.cs index e2f12c3f..543636d8 100644 --- a/MetadataExtractor/IO/BufferReader.Indexed.cs +++ b/MetadataExtractor/IO/BufferReader.Indexed.cs @@ -178,6 +178,9 @@ public readonly double GetDouble64(int index) public readonly string GetString(int index, int bytesRequested, Encoding encoding) { + if (bytesRequested < 0) + throw new ArgumentOutOfRangeException(nameof(bytesRequested), "Must be 0 or greater"); + // This check is important on .NET Framework if (bytesRequested is 0) { diff --git a/MetadataExtractor/IO/BufferReader.Sequential.cs b/MetadataExtractor/IO/BufferReader.Sequential.cs index 1231664c..17256742 100644 --- a/MetadataExtractor/IO/BufferReader.Sequential.cs +++ b/MetadataExtractor/IO/BufferReader.Sequential.cs @@ -114,17 +114,19 @@ public ulong GetUInt64() public string GetString(int bytesRequested, Encoding encoding) { - // This check is important on .NET Framework + if (bytesRequested < 0) + throw new ArgumentOutOfRangeException(nameof(bytesRequested), "Must be 0 or greater"); + if (bytesRequested is 0) return ""; - Span bytes = bytesRequested <= 256 - ? stackalloc byte[bytesRequested] - : new byte[bytesRequested]; + using var buffer = bytesRequested <= 256 + ? new ScopedBuffer(stackalloc byte[bytesRequested]) + : new ScopedBuffer(bytesRequested); - GetBytes(bytes); + GetBytes(buffer); - return encoding.GetString(bytes); + return encoding.GetString(buffer); } public StringValue GetNullTerminatedStringValue(int maxLengthBytes, Encoding? encoding = null, bool moveToMaxLength = false) diff --git a/MetadataExtractor/IO/IndexedReader.cs b/MetadataExtractor/IO/IndexedReader.cs index 4126d833..8ec40f75 100644 --- a/MetadataExtractor/IO/IndexedReader.cs +++ b/MetadataExtractor/IO/IndexedReader.cs @@ -320,6 +320,9 @@ public double GetDouble64(int index) /// public string GetString(int index, int bytesRequested, Encoding encoding) { + if (bytesRequested < 0) + throw new ArgumentOutOfRangeException(nameof(bytesRequested), "Must be 0 or greater"); + // This check is important on .NET Framework if (bytesRequested is 0) { diff --git a/MetadataExtractor/IO/SequentialReader.cs b/MetadataExtractor/IO/SequentialReader.cs index 055bead9..9ae1fbb0 100644 --- a/MetadataExtractor/IO/SequentialReader.cs +++ b/MetadataExtractor/IO/SequentialReader.cs @@ -221,6 +221,9 @@ public float GetS15Fixed16() /// public string GetString(int bytesRequested, Encoding encoding) { + if (bytesRequested < 0) + throw new ArgumentOutOfRangeException(nameof(bytesRequested), "Must be 0 or greater"); + // This check is important on .NET Framework if (bytesRequested is 0) { From a436552568dba3fa97ecab9f491a6532d4e1bb25 Mon Sep 17 00:00:00 2001 From: Jason Nelson Date: Wed, 21 Feb 2024 22:50:05 -0800 Subject: [PATCH 5/8] Remove unused using statements --- MetadataExtractor/IO/BufferReader.Indexed.cs | 1 - MetadataExtractor/IO/IndexedReader.cs | 2 -- MetadataExtractor/IO/SequentialReader.cs | 1 - 3 files changed, 4 deletions(-) diff --git a/MetadataExtractor/IO/BufferReader.Indexed.cs b/MetadataExtractor/IO/BufferReader.Indexed.cs index 543636d8..99690355 100644 --- a/MetadataExtractor/IO/BufferReader.Indexed.cs +++ b/MetadataExtractor/IO/BufferReader.Indexed.cs @@ -1,6 +1,5 @@ // Copyright (c) Drew Noakes and contributors. All Rights Reserved. Licensed under the Apache License, Version 2.0. See LICENSE in the project root for license information. -using System.Buffers; using System.Buffers.Binary; namespace MetadataExtractor.IO; diff --git a/MetadataExtractor/IO/IndexedReader.cs b/MetadataExtractor/IO/IndexedReader.cs index 8ec40f75..9cc02821 100644 --- a/MetadataExtractor/IO/IndexedReader.cs +++ b/MetadataExtractor/IO/IndexedReader.cs @@ -1,7 +1,5 @@ // Copyright (c) Drew Noakes and contributors. All Rights Reserved. Licensed under the Apache License, Version 2.0. See LICENSE in the project root for license information. - -using System.Buffers; using System.Buffers.Binary; namespace MetadataExtractor.IO diff --git a/MetadataExtractor/IO/SequentialReader.cs b/MetadataExtractor/IO/SequentialReader.cs index 9ae1fbb0..2d616138 100644 --- a/MetadataExtractor/IO/SequentialReader.cs +++ b/MetadataExtractor/IO/SequentialReader.cs @@ -1,6 +1,5 @@ // Copyright (c) Drew Noakes and contributors. All Rights Reserved. Licensed under the Apache License, Version 2.0. See LICENSE in the project root for license information. -using System.Buffers; using System.Buffers.Binary; namespace MetadataExtractor.IO From 52c9b1fa7ef7a733d1b183c74d36f4ade47508b0 Mon Sep 17 00:00:00 2001 From: Jason Nelson Date: Wed, 21 Feb 2024 22:57:15 -0800 Subject: [PATCH 6/8] Use NET8_0_OR_GREATER --- MetadataExtractor/Util/DateUtil.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/MetadataExtractor/Util/DateUtil.cs b/MetadataExtractor/Util/DateUtil.cs index b29aae88..98d9fda3 100644 --- a/MetadataExtractor/Util/DateUtil.cs +++ b/MetadataExtractor/Util/DateUtil.cs @@ -18,7 +18,7 @@ public static bool IsValidTime(int hours, int minutes, int seconds) minutes is >= 0 and < 60 && seconds is >= 0 and < 60; -#if NET8_0 || NETSTANDARD2_1 +#if NET8_0_OR_GREATER || NETSTANDARD2_1 private static readonly DateTime _unixEpoch = DateTime.UnixEpoch; #else private static readonly DateTime _unixEpoch = new(1970, 1, 1, 0, 0, 0); From 337d235627c51da5ea6eb4a86832fbed85cb2304 Mon Sep 17 00:00:00 2001 From: Jason Nelson Date: Thu, 22 Feb 2024 20:57:56 -0800 Subject: [PATCH 7/8] Unify GetString logic --- MetadataExtractor/IO/BufferReader.Indexed.cs | 20 ++++++------------- .../IO/BufferReader.Sequential.cs | 5 +++-- MetadataExtractor/IO/IndexedReader.cs | 20 ++++++------------- MetadataExtractor/IO/ScopedBuffer.cs | 4 ++-- MetadataExtractor/IO/SequentialReader.cs | 20 ++++++------------- 5 files changed, 23 insertions(+), 46 deletions(-) diff --git a/MetadataExtractor/IO/BufferReader.Indexed.cs b/MetadataExtractor/IO/BufferReader.Indexed.cs index 99690355..0eb25066 100644 --- a/MetadataExtractor/IO/BufferReader.Indexed.cs +++ b/MetadataExtractor/IO/BufferReader.Indexed.cs @@ -178,29 +178,21 @@ public readonly double GetDouble64(int index) public readonly string GetString(int index, int bytesRequested, Encoding encoding) { if (bytesRequested < 0) - throw new ArgumentOutOfRangeException(nameof(bytesRequested), "Must be 0 or greater"); + throw new ArgumentOutOfRangeException(nameof(bytesRequested), "Must be zero or greater."); // This check is important on .NET Framework if (bytesRequested is 0) { return ""; } - else if (bytesRequested < 256) - { - Span bytes = stackalloc byte[bytesRequested]; - - GetBytes(index, bytes); - return encoding.GetString(bytes); - } - else - { - using var buffer = new ScopedBuffer(bytesRequested); + using var buffer = bytesRequested <= ScopedBuffer.MaxStackBufferSize + ? new ScopedBuffer(stackalloc byte[bytesRequested]) + : new ScopedBuffer(bytesRequested); - GetBytes(index, buffer); + GetBytes(index, buffer); - return encoding.GetString(buffer); - } + return encoding.GetString(buffer); } private readonly void ValidateIndex(int index, int bytesRequested) diff --git a/MetadataExtractor/IO/BufferReader.Sequential.cs b/MetadataExtractor/IO/BufferReader.Sequential.cs index 17256742..4c8bbc2d 100644 --- a/MetadataExtractor/IO/BufferReader.Sequential.cs +++ b/MetadataExtractor/IO/BufferReader.Sequential.cs @@ -115,12 +115,13 @@ public ulong GetUInt64() public string GetString(int bytesRequested, Encoding encoding) { if (bytesRequested < 0) - throw new ArgumentOutOfRangeException(nameof(bytesRequested), "Must be 0 or greater"); + throw new ArgumentOutOfRangeException(nameof(bytesRequested), "Must be zero or greater."); + // This check is important on .NET Framework if (bytesRequested is 0) return ""; - using var buffer = bytesRequested <= 256 + using var buffer = bytesRequested <= ScopedBuffer.MaxStackBufferSize ? new ScopedBuffer(stackalloc byte[bytesRequested]) : new ScopedBuffer(bytesRequested); diff --git a/MetadataExtractor/IO/IndexedReader.cs b/MetadataExtractor/IO/IndexedReader.cs index 9cc02821..2d4a0650 100644 --- a/MetadataExtractor/IO/IndexedReader.cs +++ b/MetadataExtractor/IO/IndexedReader.cs @@ -319,29 +319,21 @@ public double GetDouble64(int index) public string GetString(int index, int bytesRequested, Encoding encoding) { if (bytesRequested < 0) - throw new ArgumentOutOfRangeException(nameof(bytesRequested), "Must be 0 or greater"); + throw new ArgumentOutOfRangeException(nameof(bytesRequested), "Must be zero or greater."); // This check is important on .NET Framework if (bytesRequested is 0) { return ""; } - else if (bytesRequested < 256) - { - Span bytes = stackalloc byte[bytesRequested]; - - GetBytes(index, bytes); - return encoding.GetString(bytes); - } - else - { - using var buffer = new ScopedBuffer(bytesRequested); + using var buffer = bytesRequested <= ScopedBuffer.MaxStackBufferSize + ? new ScopedBuffer(stackalloc byte[bytesRequested]) + : new ScopedBuffer(bytesRequested); - GetBytes(index, buffer); + GetBytes(index, buffer); - return encoding.GetString(buffer); - } + return encoding.GetString(buffer); } /// diff --git a/MetadataExtractor/IO/ScopedBuffer.cs b/MetadataExtractor/IO/ScopedBuffer.cs index eddae690..ee935851 100644 --- a/MetadataExtractor/IO/ScopedBuffer.cs +++ b/MetadataExtractor/IO/ScopedBuffer.cs @@ -6,6 +6,8 @@ namespace MetadataExtractor.IO; internal ref struct ScopedBuffer { + public const int MaxStackBufferSize = 256; + private byte[]? _rentedBuffer; private readonly Span _span; @@ -37,7 +39,5 @@ public void Dispose() if (_rentedBuffer is null) return; ArrayPool.Shared.Return(_rentedBuffer); - - _rentedBuffer = null; } } diff --git a/MetadataExtractor/IO/SequentialReader.cs b/MetadataExtractor/IO/SequentialReader.cs index 2d616138..e0e50974 100644 --- a/MetadataExtractor/IO/SequentialReader.cs +++ b/MetadataExtractor/IO/SequentialReader.cs @@ -221,29 +221,21 @@ public float GetS15Fixed16() public string GetString(int bytesRequested, Encoding encoding) { if (bytesRequested < 0) - throw new ArgumentOutOfRangeException(nameof(bytesRequested), "Must be 0 or greater"); + throw new ArgumentOutOfRangeException(nameof(bytesRequested), "Must be zero or greater."); // This check is important on .NET Framework if (bytesRequested is 0) { return ""; } - else if (bytesRequested < 256) - { - Span bytes = stackalloc byte[bytesRequested]; - - GetBytes(bytes); - return encoding.GetString(bytes); - } - else - { - using var buffer = new ScopedBuffer(bytesRequested); + using var buffer = bytesRequested <= ScopedBuffer.MaxStackBufferSize + ? new ScopedBuffer(stackalloc byte[bytesRequested]) + : new ScopedBuffer(bytesRequested); - GetBytes(buffer); + GetBytes(buffer); - return encoding.GetString(buffer); - } + return encoding.GetString(buffer); } public StringValue GetStringValue(int bytesRequested, Encoding? encoding = null) From 50d44e86ea260993e67a7138c1c7cf34a89ee63b Mon Sep 17 00:00:00 2001 From: Jason Nelson Date: Thu, 22 Feb 2024 20:59:06 -0800 Subject: [PATCH 8/8] Clarify count range --- MetadataExtractor/Formats/Apple/BplistReader.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/MetadataExtractor/Formats/Apple/BplistReader.cs b/MetadataExtractor/Formats/Apple/BplistReader.cs index 320ed773..f59e0b3e 100644 --- a/MetadataExtractor/Formats/Apple/BplistReader.cs +++ b/MetadataExtractor/Formats/Apple/BplistReader.cs @@ -121,7 +121,7 @@ static object HandleInt(ref BufferReader reader, byte marker) static Dictionary HandleDict(ref BufferReader reader, byte count) { - Span keyRefs = stackalloc byte[count]; // <= 255 + Span keyRefs = stackalloc byte[count]; // count is a byte (0-255) reader.GetBytes(keyRefs);