CurtHagenlocher commented on code in PR #2669: URL: https://github.com/apache/arrow-adbc/pull/2669#discussion_r2029225879
########## csharp/src/Drivers/Apache/Spark/Lz4Utilities.cs: ########## @@ -0,0 +1,57 @@ +/* +* Licensed to the Apache Software Foundation (ASF) under one or more +* contributor license agreements. See the NOTICE file distributed with +* this work for additional information regarding copyright ownership. +* The ASF licenses this file to You under the Apache License, Version 2.0 +* (the "License"); you may not use this file except in compliance with +* the License. You may obtain a copy of the License at +* +* http://www.apache.org/licenses/LICENSE-2.0 +* +* Unless required by applicable law or agreed to in writing, software +* distributed under the License is distributed on an "AS IS" BASIS, +* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +* See the License for the specific language governing permissions and +* limitations under the License. +*/ + +using System; +using System.Buffers; +using System.IO; +using K4os.Compression.LZ4.Streams; + +namespace Apache.Arrow.Adbc.Drivers.Apache.Spark +{ + /// <summary> + /// Utility class for LZ4 compression/decompression operations. + /// </summary> + internal static class Lz4Utilities + { + /// <summary> + /// Decompresses LZ4 compressed data into memory. + /// </summary> + /// <param name="compressedData">The compressed data bytes.</param> + /// <returns>A ReadOnlyMemory containing the decompressed data.</returns> + /// <exception cref="AdbcException">Thrown when decompression fails.</exception> + public static ReadOnlyMemory<byte> DecompressLz4(byte[] compressedData) + { + try + { + var outputStream = new MemoryStream(); + using (var inputStream = new MemoryStream(compressedData)) + using (var decompressor = LZ4Stream.Decode(inputStream)) + { + decompressor.CopyTo(outputStream); + } + // Get the underlying buffer and its valid length without copying + return new ReadOnlyMemory<byte>(outputStream.GetBuffer(), 0, (int)outputStream.Length); + // Note: We're not disposing the outputStream here because we're returning its buffer. + // The memory will be reclaimed when the ReadOnlyMemory is no longer referenced. Review Comment: I don't know that we necessarily need to remove or change the comment, but this isn't strictly correct. There are a lot of misunderstandings about `IDisposable`, but disposing a `MemoryStream` does nothing to its internal buffer. Because the buffer is a managed array, it will not get garbage-collected until there are no more references to it -- even if the `MemoryStream` that created it is disposed. ########## csharp/src/Drivers/Apache/Spark/Lz4Utilities.cs: ########## @@ -0,0 +1,57 @@ +/* +* Licensed to the Apache Software Foundation (ASF) under one or more +* contributor license agreements. See the NOTICE file distributed with +* this work for additional information regarding copyright ownership. +* The ASF licenses this file to You under the Apache License, Version 2.0 +* (the "License"); you may not use this file except in compliance with +* the License. You may obtain a copy of the License at +* +* http://www.apache.org/licenses/LICENSE-2.0 +* +* Unless required by applicable law or agreed to in writing, software +* distributed under the License is distributed on an "AS IS" BASIS, +* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +* See the License for the specific language governing permissions and +* limitations under the License. +*/ + +using System; +using System.Buffers; +using System.IO; +using K4os.Compression.LZ4.Streams; + +namespace Apache.Arrow.Adbc.Drivers.Apache.Spark +{ + /// <summary> + /// Utility class for LZ4 compression/decompression operations. + /// </summary> + internal static class Lz4Utilities + { + /// <summary> + /// Decompresses LZ4 compressed data into memory. + /// </summary> + /// <param name="compressedData">The compressed data bytes.</param> + /// <returns>A ReadOnlyMemory containing the decompressed data.</returns> + /// <exception cref="AdbcException">Thrown when decompression fails.</exception> + public static ReadOnlyMemory<byte> DecompressLz4(byte[] compressedData) + { + try + { + var outputStream = new MemoryStream(); + using (var inputStream = new MemoryStream(compressedData)) + using (var decompressor = LZ4Stream.Decode(inputStream)) + { + decompressor.CopyTo(outputStream); + } + // Get the underlying buffer and its valid length without copying + return new ReadOnlyMemory<byte>(outputStream.GetBuffer(), 0, (int)outputStream.Length); + // Note: We're not disposing the outputStream here because we're returning its buffer. + // The memory will be reclaimed when the ReadOnlyMemory is no longer referenced. Review Comment: It's fair to not want to assume anything about what `MemoryStream.Dispose` might do to its internal buffer, but we're now assuming instead that a `MemoryStream` doesn't need to be disposed. (This happens to be true.) -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: github-unsubscr...@arrow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
