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
Other information
No response
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
Relevant log output
Other information
No response