You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@opendal.apache.org by "Fatorin (via GitHub)" <gi...@apache.org> on 2026/08/04 06:24:14 UTC
[PR] refactor(bindings/dotnet): rework the FFI boundary and add zero-copy IO paths [opendal]
Fatorin opened a new pull request, #8020:
URL: https://github.com/apache/opendal/pull/8020
# Which issue does this PR close?
<!--
We generally require a GitHub issue to be filed for all bug fixes and enhancements. You can link an issue to this PR using the GitHub syntax. For example `Closes #123` indicates that this PR will close issue #123.
-->
None
# Rationale for this change
<!--
Why are you proposing this change? If this is already explained clearly in the issue then this section is not needed.
Explaining clearly why changes are proposed helps reviewers understand your changes and offer better suggestions for fixes.
-->
This PR removes the redundant copies from every read and write path, makes the stream async methods truly async, and fixes a silent memory-corruption hazard in the sync write path. It started as performance tuning, but the tuning kept exposing rough spots in the earlier implementation, so the boundary cleanup rides along:
- The sync write hands backends the caller's array without a copy. An in-process backend keeps reading that memory after the call returns, so later changes to the array silently corrupt what the backend stored.
- Every payload is copied more often than it needs to be.
- The stream async methods block on the sync natives.
- The read FFI names its buffers just `buffer_*`. This PR moves buffers across the boundary in both directions, and a name that omits the direction also hides the contract: which side fills the memory and which side frees it.
# What changes are included in this PR?
<!--
There is no need to duplicate the description in the issue here but it is sometimes worth providing a summary of the individual changes in this PR.
-->
The bullets follow the commit order — the zero-copy commit gets two.
- **List entries marshal in one pass.** Store them by value in one contiguous array instead of a pointer chase per entry.
- **Executor registry lookups stop serializing.** Swap the registry `Mutex` for an `RwLock`.
- **Zero-copy callback paths in both directions.** A write callback fills native memory through `IBufferWriter<byte>` (the .NET counterpart of `bytes::BufMut`), and the write consumes those segments without copying. A read callback parses the native chunks in place through `ReadOnlySequence<byte>` (the .NET counterpart of `opendal::Buffer`). Stream reads become single-copy: each native chunk goes straight into the caller's buffer. The buffers behind all of this stay internal to the binding.
- **The `byte[]` overloads keep their signatures and copy once at the boundary.** A write copies the array into native memory during the call, so the native side no longer keeps a pointer into the caller's array after the call returns. A read assembles the returned array from the native chunks. The write-side copy is what fixes the corruption described in the rationale, and a regression test proves it by mutating the source array right after a write.
- **Streams are truly async and explicitly completable.** The async methods drive the native async reader and writer instead of blocking, and `Complete`/`CompleteAsync` let callers finalize a write and see its errors instead of relying on disposal (the .NET destructor path).
- **The interop boundary tightens.** `Operator.Op` and `AsyncState<T>` leave the public surface, internal-only helpers stop being exported symbols, a dead import goes away, and the read FFI family is renamed `read_buffer_*` to mirror the new `write_buffer_*` one.
# Are there any user-facing changes?
<!--
If there are user-facing changes then we may require documentation to be updated before approving the PR.
-->
<!---
If there are any breaking changes to public APIs, please add the `breaking-changes` label.
-->
### API changes
| change | API | notes |
| --- | --- | --- |
| Added | `Write` / `WriteAsync(path, Action<IBufferWriter<byte>>, int sizeHint = 0)` | The producer fills a native-backed writer. `sizeHint` pre-sizes the first native segment. The memory the writer hands out is uninitialized, as pooled buffers usually are, so write before you read. |
| Added | `Read<T>` / `ReadAsync<T>(path, Func<ReadOnlySequence<byte>, T>)` | The consumer parses the payload in place over native memory. |
| Added | `OperatorOutputStream.Complete` / `CompleteAsync` | Flush, close the native writer, and seal the stream, so write errors surface instead of being lost in disposal. |
| Removed | `Operator.Op` | It duplicated the raw-handle accessor (`SafeHandle.DangerousGetHandle`) that is already public. |
| Removed | `AsyncState<T>` | Now internal. Nothing outside the library has a use for the callback plumbing. |
The new surface speaks only standard-library buffer types (`System.Buffers`), no binding-invented ones, so standard producers and parsers work over native memory directly, and that memory never outlives the callback scope.
### Compatibility
Existing `byte[]` code keeps working as before. Measured against main on the same rig and methodology as the benchmarks below, `byte[]` read and write throughput is unchanged within noise on every service, small writes included.
Cancellation semantics on streams got stricter: a canceled in-flight stream read or flush poisons the stream, and later use throws instead of silently skipping or duplicating bytes. Disposing a poisoned stream still releases its native resources but skips the final flush and close. One-shot reads and writes stay safe to cancel — an abandoned native result is released by the completion callback.
### Benchmarks
AMD Ryzen 5 9600X (6C/12T), 32 GB DDR5-4800, Windows 11, release builds on both sides, Stopwatch medians. Services are the in-process memory service, local fs, and webdav backed by dufs, all over loopback. Only the stream table is a before/after against main. The other tables compare the two APIs this PR ships — the `byte[]` overloads against the callback forms, measured back to back within every iteration so service drift hits both equally — and read as a guide to choosing between them, not as a regression check.
One caveat: a real .NET application would reach for `MemoryCache` (an in-process cache) rather than the memory service, so the memory rows mostly isolate what the FFI boundary costs, while the storage-backed services are the realistic scenarios.
**Stream read** — 16 MiB payload, MiB/s. In-process throughput nearly quintuples, fs more than doubles, the HTTP-bound webdav holds steady (medians of five interleaved before/after rounds, to keep HTTP drift out of the comparison), and GC allocations per pass drop from the payload size to 128 B on every service:
| service | before | after |
| --- | --- | --- |
| memory | 14,401 | 66,170 |
| fs | 3,263 | 7,651 |
| webdav (dufs) | 634 | 637 |
**Read and consume** — 16 MiB payload, MiB/s. The `byte[]` overload materializes a managed array, the sequence callback parses in place:
| service | Read (byte[]) | Read (sequence callback) | allocations per read, byte[] -> callback |
| --- | --- | --- | --- |
| memory | 3,441 | 4,879 | 16.8 MB -> 956 B |
| fs | 1,503 | 1,794 | 16.8 MB -> 1.7 KB |
| webdav (dufs) | 476 | 522 | 16.8 MB -> 46 KB |
**Fill and write** — 16 MiB payload, MiB/s, each column timing the full fill plus the write. The `byte[]` overload fills a managed array first, the fill callback writes straight into native memory:
| service | Write (byte[]) | Write (fill callback) |
| --- | --- | --- |
| memory | 4,950 | 6,927 |
| fs | 702 | 747 |
| webdav (dufs) | 92 | 91 |
**Payload size scaling** — MiB/s medians of the same two-API comparison, the 16 MiB rows repeating the tables above. The step to 128 MiB changes nothing about the picture: the callback read keeps its lead on memory and fs while the `byte[]` overload pays the GC for a payload-sized array on every call, webdav stays wire-bound with the write columns converged, and the allocation profiles hold (callback reads well under the payload size, stream reads at 128 B per pass):
| service | payload | Read (byte[]) | Read (callback) | Write (byte[]) | Write (callback) |
| --- | --- | --- | --- | --- | --- |
| memory | 16 MiB | 3,441 | 4,879 | 4,950 | 6,927 |
| memory | 128 MiB | 3,068 | 4,706 | 3,490 | 5,289 |
| fs | 16 MiB | 1,503 | 1,794 | 702 | 747 |
| fs | 128 MiB | 1,363 | 1,648 | 686 | 712 |
| webdav (dufs) | 16 MiB | 476 | 522 | 92 | 91 |
| webdav (dufs) | 128 MiB | 457 | 502 | 86 | 88 |
# AI Usage Statement
<!--
If you are using AI tools to build this PR, please include a statement specifying the tool and models you are using.
-->
This PR was developed with Claude Code (model: Claude Fable 5).
--
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: commits-unsubscribe@opendal.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] refactor(bindings/dotnet): rework the FFI boundary and add zero-copy IO paths [opendal]
Posted by "Xuanwo (via GitHub)" <gi...@apache.org>.
Xuanwo merged PR #8020:
URL: https://github.com/apache/opendal/pull/8020
--
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: commits-unsubscribe@opendal.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] refactor(bindings/dotnet): rework the FFI boundary and add zero-copy IO paths [opendal]
Posted by "Fatorin (via GitHub)" <gi...@apache.org>.
Fatorin commented on PR #8020:
URL: https://github.com/apache/opendal/pull/8020#issuecomment-5183777330
> It's kind that Claude Fable 5 produced 7 commits for review 😢 Will hopefully read in this week!
I'm sorry for making this pull request too complex and hard to review.
I should have separated the zero-copy read and write into another pull request.
The other parts are just tiny changes for renaming class names and improving performance a bit.
--
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: commits-unsubscribe@opendal.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] refactor(bindings/dotnet): rework the FFI boundary and add zero-copy IO paths [opendal]
Posted by "erickguan (via GitHub)" <gi...@apache.org>.
erickguan commented on PR #8020:
URL: https://github.com/apache/opendal/pull/8020#issuecomment-5186311149
No worries. I will get to it.
It's also nice to get some performance numbers. I remembered .NET core improved file and stream I/O a lot a few years ago.
--
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: commits-unsubscribe@opendal.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] refactor(bindings/dotnet): rework the FFI boundary and add zero-copy IO paths [opendal]
Posted by "Fatorin (via GitHub)" <gi...@apache.org>.
Fatorin commented on PR #8020:
URL: https://github.com/apache/opendal/pull/8020#issuecomment-5176042763
> It's not a blocker but I'm intereted in how opendal bindings' performance compared with dotnet's native IO, is there a number?
I made a repository to run the benchmarks: https://github.com/Fatorin/opendal-dotnet-io-bench
Basically .NET native IO is much faster on smaller payloads, but the gap narrows as the file grows. Some numbers from my machine:
At 16 KiB the binding pays a fixed ~0.1-0.4 ms per operation (FFI plus async-runtime dispatch), so .NET's native IO wins by 2.6x-14x.
At 128 MiB stream reads actually beat FileStream (0.91x) and writes converge to about 1.4x.
Full tables with allocation columns are in the README.
--
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: commits-unsubscribe@opendal.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] refactor(bindings/dotnet): rework the FFI boundary and add zero-copy IO paths [opendal]
Posted by "erickguan (via GitHub)" <gi...@apache.org>.
erickguan commented on code in PR #8020:
URL: https://github.com/apache/opendal/pull/8020#discussion_r3717305249
##########
bindings/dotnet/src/executor.rs:
##########
@@ -29,8 +29,8 @@ use crate::utils::config_invalid_error;
static DEFAULT_EXECUTOR: OnceLock<Arc<Executor>> = OnceLock::new();
-static EXECUTOR_REGISTRY: LazyLock<Mutex<HashMap<usize, Arc<Executor>>>> =
- LazyLock::new(|| Mutex::new(HashMap::new()));
+static EXECUTOR_REGISTRY: LazyLock<RwLock<HashMap<usize, Arc<Executor>>>> =
Review Comment:
Maybe a comment documenting this is for prevent serialization.
I have a hard time understanding 2 event loops mentally. And also, can we use .NET's runtime for executor? It's an idea.
--
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: commits-unsubscribe@opendal.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] refactor(bindings/dotnet): rework the FFI boundary and add zero-copy IO paths [opendal]
Posted by "Xuanwo (via GitHub)" <gi...@apache.org>.
Xuanwo commented on PR #8020:
URL: https://github.com/apache/opendal/pull/8020#issuecomment-5182952783
Thank you @Fatorin for the bench! Can we also test over s3? (minio is enough)
--
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: commits-unsubscribe@opendal.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] refactor(bindings/dotnet): rework the FFI boundary and add zero-copy IO paths [opendal]
Posted by "erickguan (via GitHub)" <gi...@apache.org>.
erickguan commented on code in PR #8020:
URL: https://github.com/apache/opendal/pull/8020#discussion_r3718045953
##########
bindings/dotnet/OpenDAL/Interop/Buffers/WriteBuffer.cs:
##########
@@ -0,0 +1,165 @@
+/*
+ * 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.Buffers;
+using OpenDAL.Interop.NativeObject;
+using OpenDAL.Interop.Result;
+
+namespace OpenDAL.Interop.Buffers;
+
+/// <summary>
+/// Native <see cref="IBufferWriter{T}"/> whose contents transfer to the
+/// OpenDAL layer when a write consumes them, without copying.
+/// </summary>
+/// <remarks>
+/// Allocate with <see cref="Operator.AllocateWriteBuffer"/>, produce through the
+/// standard <see cref="IBufferWriter{T}"/> contract (for example with
+/// <c>Utf8JsonWriter</c>), then hand it to a write. The write consumes the
+/// buffer: every later access throws. Growth adds native segments that never
+/// move, so earlier memory stays valid until <see cref="Dispose"/>. Instances
+/// are not thread-safe; disposing an unconsumed buffer returns the memory.
+/// </remarks>
+internal sealed class WriteBuffer : IBufferWriter<byte>, IDisposable
+{
+ internal const int DefaultInitialCapacity = 64 * 1024;
Review Comment:
nitpick: add inline comments on the unit.
--
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: commits-unsubscribe@opendal.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] refactor(bindings/dotnet): rework the FFI boundary and add zero-copy IO paths [opendal]
Posted by "Fatorin (via GitHub)" <gi...@apache.org>.
Fatorin commented on PR #8020:
URL: https://github.com/apache/opendal/pull/8020#issuecomment-5183727629
> Thank you @Fatorin for the bench! Can we also test over s3? (minio is enough)
I compared it against AWSSDK.S3. (.NET Framework with S3)
OpenDAL performs a lot better here, faster at every size in both directions.
All methods are async, and the number in parentheses is OpenDAL time / AWSSDK time (lower is faster).
| method | 16 KiB | 16 MiB | 128 MiB |
| --- | --- | --- | --- |
| Read — AWSSDK `GetObjectAsync` | 0.85 ms | 41.1 ms | 282 ms |
| Read — OpenDAL `ReadAsync` (byte[]) | 0.69 ms (0.82) | 20.5 ms (0.50) | 148 ms (0.52) |
| Read — OpenDAL `ReadAsync` (sequence callback) | 0.69 ms (0.82) | 17.9 ms (0.44) | 136 ms (0.48) |
| Write — AWSSDK `PutObjectAsync` | 7.1 ms | 104 ms | 919 ms |
| Write — OpenDAL `WriteAsync` (byte[]) | 4.9 ms (0.71) | 56 ms (0.55) | 766 ms (0.87) |
| Write — OpenDAL `WriteAsync` (fill callback) | 5.0 ms (0.72) | 66 ms (0.64) | 759 ms (0.86) |
The SDK allocates about 2.4x the payload per read and several MB per big write, while opendal stays in the KB range with the callback read and under 2 KB for writes.
The 128 MiB writes on MinIO are pretty noisy (±25-35%), so those two ratios are rough.
--
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: commits-unsubscribe@opendal.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] refactor(bindings/dotnet): rework the FFI boundary and add zero-copy IO paths [opendal]
Posted by "Xuanwo (via GitHub)" <gi...@apache.org>.
Xuanwo commented on PR #8020:
URL: https://github.com/apache/opendal/pull/8020#issuecomment-5175568623
It's not a blocker but I'm intereted in how opendal bindings compared with dotnet's native IO, is there a number?
--
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: commits-unsubscribe@opendal.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] refactor(bindings/dotnet): rework the FFI boundary and add zero-copy IO paths [opendal]
Posted by "erickguan (via GitHub)" <gi...@apache.org>.
erickguan commented on PR #8020:
URL: https://github.com/apache/opendal/pull/8020#issuecomment-5181389086
It's kind that Claude Fable 5 produced 7 commits for review 😢 Will hopefully read in this week!
--
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: commits-unsubscribe@opendal.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] refactor(bindings/dotnet): rework the FFI boundary and add zero-copy IO paths [opendal]
Posted by "Fatorin (via GitHub)" <gi...@apache.org>.
Fatorin commented on code in PR #8020:
URL: https://github.com/apache/opendal/pull/8020#discussion_r3718502594
##########
bindings/dotnet/src/executor.rs:
##########
@@ -29,8 +29,8 @@ use crate::utils::config_invalid_error;
static DEFAULT_EXECUTOR: OnceLock<Arc<Executor>> = OnceLock::new();
-static EXECUTOR_REGISTRY: LazyLock<Mutex<HashMap<usize, Arc<Executor>>>> =
- LazyLock::new(|| Mutex::new(HashMap::new()));
+static EXECUTOR_REGISTRY: LazyLock<RwLock<HashMap<usize, Arc<Executor>>>> =
Review Comment:
Oh, it's a good idea.
When I was following other bindings I added an unnecessary design here, they just hand the raw pointer to the host side, so the registry can go away.
I will change it in a follow-up PR.
--
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: commits-unsubscribe@opendal.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org