ihudedi opened a new issue, #884:
URL: https://github.com/apache/mina-sshd/issues/884
### Version
2.15.0
### Bug description
Problem Summary
LocalWindow.release() becomes a significant performance bottleneck during
SFTP uploads, being called hundreds of times per second with excessive
overhead. This causes SFTP upload speeds to be significantly slower than in
version 2.13.2.
### Actual behavior
1.Too aggressive window adjustment threshold
Location: Line 110
if (released > packetSize / 2 || released > maxFree / 10 || released > 16 *
1024)
Problem: The 16KB threshold is too low for modern high-throughput
connections. This causes window adjust messages to be sent very frequently,
adding network and processing overhead.
Impact: With 64KB packets, this sends window adjusts for every ~2-3 packets,
generating excessive SSH_MSG_CHANNEL_WINDOW_ADJUST messages.
2.Complex shouldAdjust logic causes unnecessary adjustments
Location: Line 114
if (size < maxFree / 2 || maxFree - size > 3 * packetSize)
Problem: The condition maxFree - size > 3 * packetSize causes window
adjustments even when the window is not actually low. This is overly aggressive
and sends adjustments "just because space is available" rather than because the
sender needs it.
Impact: Generates unnecessary network messages and processing overhead.
3.Redundant AtomicLong inside synchronized block
Location: Line 42, 118
private final AtomicLong adjustment = new AtomicLong();// Later in
release():long adjustSize = adjustment.addAndGet(newSize - size); // Inside
synchronized(lock)
Problem: Using AtomicLong operations inside a synchronized(lock) block is
redundant. The lock already provides thread safety, so the atomic operations
add unnecessary overhead for every window adjustment.
Impact: Every window adjust performs expensive atomic compare-and-swap
operations when a simple long addition would suffice
4.No early exit optimization
Problem: The method always enters the synchronized block and performs
calculations even when below the threshold and no adjustment will be sent.
Impact: Adds synchronization overhead for every small chunk release, even
when no action is needed
### Expected behavior
1.Minimal per-call overhead: LocalWindow.release() should have minimal
CPU overhead since it's called very frequently during data transfer (hundreds
of times per second in proxy scenarios).
2.Efficient window management: Window adjust messages
(SSH_MSG_CHANNEL_WINDOW_ADJUST) should only be sent when the window is actually
getting low and the sender needs more space, not just because space is
available.
3.Appropriate threshold for high-throughput: The threshold for
triggering window adjustments should scale with packet size to avoid excessive
messages in high-throughput scenarios (e.g., 128KB instead of 16KB).
4.No redundant synchronization: Should not use atomic operations inside
synchronized blocks where the lock already provides thread safety.
5.Performance comparable to 2.13.2: SFTP transfer speeds should not
regress significantly from previous versions. In our testing, 2.15.0 was
notably slower than 2.13.2 for the same workload.
6.Efficient for proxy architectures: Should perform well when data is
being forwarded between channels (proxy/gateway scenarios) where release() is
called very frequently on multiple LocalWindow instances simultaneously.
### Relevant log output
```Shell
```
### Other information
_No response_
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
