Repository navigation
Conversation
When followSymbolicLinks is false, uploadDirectory now resolves each file below the source directory without following symbolic links every time it reads the file, not only when it lists the directory. - Add ConfinedFileAccess, which resolves a path below a root one component at a time without following links. - Add an internal confineToRoot option to FileAsyncRequestBody, and keep it on split parts. - Pass the source directory as the root from UploadDirectoryHelper unless links are followed or the transformer changed the source.
|
I was apprehensive about perf of On a beefy and on a much smaller No evidence of any appreciable slowdown. FWIW on Linux and Mac, |
| private static final class ConfinedReadPool { | ||
| private static final int MAX_THREADS = Math.max(2, Runtime.getRuntime().availableProcessors()); | ||
| private static final ThreadPoolExecutor EXECUTOR = createExecutor(); | ||
|
|
There was a problem hiding this comment.
AsynchronousFileChannel uses unbounded cache pool iirc. Wondering if this could cause issues for smaller hosts (2-vCPU host for example) since after the change, only 2 reads would be in flight. TransferListener.bytesTransferred also runs on those threads, right? Should we use a cached pool or a much higher bound instead?
There was a problem hiding this comment.
Let me run more test with a cached pool.
From the testing I did (see above), c9g.large is 2 vCPU and did not see any difference
There was a problem hiding this comment.
Increased the max pool size, but kept it bounded. When the callbacks don't do any work, the 2 threads is actually fastest in terms of throughput. If there are callbacks that consume time but not CPU, adding threads helps, but bounded seems to do better than cached.
Results below
Throughput, MB/s (higher is better)
Small: 1000 × 64 KiB
| Calls | Bounded, 2 threads (PR default) | AsynchronousFileChannel (follow=true) | Bounded, 64 threads | Cached (unbounded) |
|---|---|---|---|---|
| 1 | 601 | 537 | 589 | 514 |
| 4 | 594 | 534 | 583 | 489 |
| 16 | 415 | 390 | 409 | 369 |
| 64 | 269 | 263 | 259 | 250 |
Medium: 100 × 1 MiB
| Calls | Bounded, 2 threads (PR default) | AsynchronousFileChannel (follow=true) | Bounded, 64 threads | Cached (unbounded) |
|---|---|---|---|---|
| 1 | 1,634 | 1,427 | 1,613 | 1,417 |
| 4 | 1,460 | 1,225 | 1,414 | 1,206 |
| 16 | 1,379 | 1,101 | 1,275 | 1,099 |
| 64 | 1,137 | 921 | 1,037 | 918 |
Large: 8 × 32 MiB (multipart)
| Calls | Bounded, 2 threads (PR default) | AsynchronousFileChannel (follow=true) | Bounded, 64 threads | Cached (unbounded) |
|---|---|---|---|---|
| 1 | 1,898 | 1,637 | 1,848 | 1,629 |
| 4 | 1,861 | 1,586 | 1,876 | 1,603 |
| 16 | 1,591 | 1,378 | 1,564 | 1,382 |
| 64 | 1,501 | 1,267 | 1,460 | 1,259 |
Blocking listener: 200 µs sleep per 16 KiB
The 2-thread pool is pinned at 109-125 MB/s, below the theoretical 164 MB/s ceiling once per-chunk overhead is included.
Both threads are busy all the time (active mean 2.0) with 381-496 reads queued on average. The 64-thread pool recovers
(3% behind to 19% ahead of AsynchronousFileChannel) and is the best variant on the medium tree.
Throughput, MB/s (higher is better)
| Calls | Bounded, 2 threads (PR default) | AsynchronousFileChannel (follow=true) | Bounded, 64 threads | Cached (unbounded) |
|---|---|---|---|---|
| small, 4 calls | 118 | 534 | 533 | 489 |
| small, 16 calls | 109 | 392 | 379 | 372 |
| medium, 4 calls | 124 | 1,052 | 1,171 | 1,051 |
| medium, 16 calls | 125 | 904 | 1,074 | 960 |
Call latency p99, seconds (lower is better)
| Calls | Bounded, 2 threads (PR default) | AsynchronousFileChannel (follow=true) | Bounded, 64 threads | Cached (unbounded) |
|---|---|---|---|---|
| small, 4 calls | 2.27 | 0.70 | 0.55 | 0.74 |
| small, 16 calls | 10.21 | 2.92 | 2.96 | 2.98 |
| medium, 4 calls | 3.44 | 0.66 | 0.58 | 0.59 |
| medium, 16 calls | 25.20 | 5.76 | 2.36 | 2.32 |
CPU-heavy listener: 200 µs spin per 16 KiB
Every variant is at 98-100% CPU and within 5% of each other: the listener consumes the CPU, and extra threads cannot
add any. Call p99 is mixed, with no consistent winner.
Throughput, MB/s (higher is better)
| Calls | Bounded, 2 threads (PR default) | AsynchronousFileChannel (follow=true) | Bounded, 64 threads | Cached (unbounded) |
|---|---|---|---|---|
| small, 4 calls | 125 | 123 | 125 | 120 |
| small, 16 calls | 113 | 116 | 116 | 114 |
| medium, 4 calls | 145 | 139 | 144 | 139 |
| medium, 16 calls | 147 | 141 | 146 | 140 |
Call latency p99, seconds (lower is better)
| Calls | Bounded, 2 threads (PR default) | AsynchronousFileChannel (follow=true) | Bounded, 64 threads | Cached (unbounded) |
|---|---|---|---|---|
| small, 4 calls | 2.37 | 2.94 | 2.87 | 3.02 |
| small, 16 calls | 9.88 | 10.09 | 9.58 | 10.13 |
| medium, 4 calls | 3.10 | 5.03 | 3.51 | 4.82 |
| medium, 16 calls | 21.61 | 16.89 | 18.32 | 17.82 |
Raise the ConfinedReadPool bound from max(2, availableProcessors) to max(64, availableProcessors). The subscriber's onNext, including any TransferListener.bytesTransferred callback, runs on the pool thread that read the chunk, so a callback that blocks holds a pool thread.
When followSymbolicLinks is false, uploadDirectory now resolves each file below the source directory without following symbolic links every time it reads the file, not only when it lists the directory.
Motivation and Context
Modifications
Testing
Screenshots (if appropriate)
Types of changes
Checklist
mvn installsucceedsscripts/new-changescript and following the instructions. Commit the new file created by the script in.changes/next-releasewith your changes.License