Skip to content

Commit bcaf354

Browse files
jscarleWojciechNagorski
jscarle
and
WojciechNagorski
authored
Significantly improve performance of ShellStream's Expect methods (#1207)
* Significantly improved performance and fixed bug with ShellStream's Expect methods. * Fix whitespace. * Fixed test. * Improve ShellStream Expect * Fixed typo. * Doubled expectBuffer's default size and added a large expect test. * Added guard clauses to ShellStream constructor and adjusted Common_CreateMoreChannelsThanMaxSessions test. * Fixed XMLDoc spelling mistakes. --------- Co-authored-by: Wojciech Nagórski <wojtpl2@gmail.com> Co-authored-by: Wojciech Nagórski <wojciech.nagorski@intel.com>
1 parent 7d357e8 commit bcaf354

File tree

24 files changed

+253
-61
lines changed

24 files changed

+253
-61
lines changed

‎src/Renci.SshNet/IServiceFactory.cs

+3-1
Original file line numberDiff line numberDiff line change
@@ -114,6 +114,7 @@ internal partial interface IServiceFactory
114114
/// <param name="height">The terminal height in pixels.</param>
115115
/// <param name="terminalModeValues">The terminal mode values.</param>
116116
/// <param name="bufferSize">Size of the buffer.</param>
117+
/// <param name="expectSize">Size of the expect buffer.</param>
117118
/// <returns>
118119
/// The created <see cref="ShellStream"/> instance.
119120
/// </returns>
@@ -135,7 +136,8 @@ ShellStream CreateShellStream(ISession session,
135136
uint width,
136137
uint height,
137138
IDictionary<TerminalModes, uint> terminalModeValues,
138-
int bufferSize);
139+
int bufferSize,
140+
int expectSize);
139141

140142
/// <summary>
141143
/// Creates an <see cref="IRemotePathTransformation"/> that encloses a path in double quotes, and escapes

‎src/Renci.SshNet/ServiceFactory.cs

+4-3
Original file line numberDiff line numberDiff line change
@@ -187,23 +187,24 @@ public ISftpResponseFactory CreateSftpResponseFactory()
187187
/// <param name="height">The terminal height in pixels.</param>
188188
/// <param name="terminalModeValues">The terminal mode values.</param>
189189
/// <param name="bufferSize">The size of the buffer.</param>
190+
/// <param name="expectSize">The size of the expect buffer.</param>
190191
/// <returns>
191192
/// The created <see cref="ShellStream"/> instance.
192193
/// </returns>
193194
/// <exception cref="SshConnectionException">Client is not connected.</exception>
194195
/// <remarks>
195196
/// <para>
196197
/// The <c>TERM</c> environment variable contains an identifier for the text window's capabilities.
197-
/// You can get a detailed list of these cababilities by using the ‘infocmp’ command.
198+
/// You can get a detailed list of these capabilities by using the ‘infocmp’ command.
198199
/// </para>
199200
/// <para>
200201
/// The column/row dimensions override the pixel dimensions(when non-zero). Pixel dimensions refer
201202
/// to the drawable area of the window.
202203
/// </para>
203204
/// </remarks>
204-
public ShellStream CreateShellStream(ISession session, string terminalName, uint columns, uint rows, uint width, uint height, IDictionary<TerminalModes, uint> terminalModeValues, int bufferSize)
205+
public ShellStream CreateShellStream(ISession session, string terminalName, uint columns, uint rows, uint width, uint height, IDictionary<TerminalModes, uint> terminalModeValues, int bufferSize, int expectSize)
205206
{
206-
return new ShellStream(session, terminalName, columns, rows, width, height, terminalModeValues, bufferSize);
207+
return new ShellStream(session, terminalName, columns, rows, width, height, terminalModeValues, bufferSize, expectSize);
207208
}
208209

209210
/// <summary>

‎src/Renci.SshNet/ShellStream.cs

+79-30
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,8 @@ public class ShellStream : Stream
2222
private readonly Encoding _encoding;
2323
private readonly int _bufferSize;
2424
private readonly Queue<byte> _incoming;
25+
private readonly int _expectSize;
26+
private readonly Queue<byte> _expect;
2527
private readonly Queue<byte> _outgoing;
2628
private IChannelSession _channel;
2729
private AutoResetEvent _dataReceived = new AutoResetEvent(initialState: false);
@@ -76,15 +78,28 @@ internal int BufferSize
7678
/// <param name="height">The terminal height in pixels.</param>
7779
/// <param name="terminalModeValues">The terminal mode values.</param>
7880
/// <param name="bufferSize">The size of the buffer.</param>
81+
/// <param name="expectSize">The size of the expect buffer.</param>
7982
/// <exception cref="SshException">The channel could not be opened.</exception>
8083
/// <exception cref="SshException">The pseudo-terminal request was not accepted by the server.</exception>
8184
/// <exception cref="SshException">The request to start a shell was not accepted by the server.</exception>
82-
internal ShellStream(ISession session, string terminalName, uint columns, uint rows, uint width, uint height, IDictionary<TerminalModes, uint> terminalModeValues, int bufferSize)
85+
internal ShellStream(ISession session, string terminalName, uint columns, uint rows, uint width, uint height, IDictionary<TerminalModes, uint> terminalModeValues, int bufferSize, int expectSize)
8386
{
87+
if (bufferSize <= 0)
88+
{
89+
throw new ArgumentException($"{nameof(bufferSize)} must be between 1 and {int.MaxValue}.");
90+
}
91+
92+
if (expectSize <= 0)
93+
{
94+
throw new ArgumentException($"{nameof(expectSize)} must be between 1 and {int.MaxValue}.");
95+
}
96+
8497
_encoding = session.ConnectionInfo.Encoding;
8598
_session = session;
8699
_bufferSize = bufferSize;
87100
_incoming = new Queue<byte>();
101+
_expectSize = expectSize;
102+
_expect = new Queue<byte>(_expectSize);
88103
_outgoing = new Queue<byte>();
89104

90105
_channel = _session.CreateChannelSession();
@@ -248,35 +263,40 @@ public void Expect(params ExpectAction[] expectActions)
248263
public void Expect(TimeSpan timeout, params ExpectAction[] expectActions)
249264
{
250265
var expectedFound = false;
251-
var text = string.Empty;
266+
var matchText = string.Empty;
252267

253268
do
254269
{
255270
lock (_incoming)
256271
{
257-
if (_incoming.Count > 0)
272+
if (_expect.Count > 0)
258273
{
259-
text = _encoding.GetString(_incoming.ToArray(), 0, _incoming.Count);
274+
matchText = _encoding.GetString(_expect.ToArray(), 0, _expect.Count);
260275
}
261276

262-
if (text.Length > 0)
277+
if (matchText.Length > 0)
263278
{
264279
foreach (var expectAction in expectActions)
265280
{
266-
var match = expectAction.Expect.Match(text);
281+
var match = expectAction.Expect.Match(matchText);
267282

268283
if (match.Success)
269284
{
270-
var result = text.Substring(0, match.Index + match.Length);
271-
var charCount = _encoding.GetByteCount(result);
285+
var returnText = matchText.Substring(0, match.Index + match.Length);
286+
var returnLength = _encoding.GetByteCount(returnText);
272287

273-
for (var i = 0; i < charCount && _incoming.Count > 0; i++)
288+
// Remove processed items from the queue
289+
for (var i = 0; i < returnLength && _incoming.Count > 0; i++)
274290
{
275-
// Remove processed items from the queue
291+
if (_expect.Count == _incoming.Count)
292+
{
293+
_ = _expect.Dequeue();
294+
}
295+
276296
_ = _incoming.Dequeue();
277297
}
278298

279-
expectAction.Action(result);
299+
expectAction.Action(returnText);
280300
expectedFound = true;
281301
}
282302
}
@@ -349,27 +369,33 @@ public string Expect(Regex regex)
349369
/// </returns>
350370
public string Expect(Regex regex, TimeSpan timeout)
351371
{
352-
var result = string.Empty;
372+
var matchText = string.Empty;
373+
string returnText;
353374

354375
while (true)
355376
{
356377
lock (_incoming)
357378
{
358-
if (_incoming.Count > 0)
379+
if (_expect.Count > 0)
359380
{
360-
result = _encoding.GetString(_incoming.ToArray(), 0, _incoming.Count);
381+
matchText = _encoding.GetString(_expect.ToArray(), 0, _expect.Count);
361382
}
362383

363-
var match = regex.Match(result);
384+
var match = regex.Match(matchText);
364385

365386
if (match.Success)
366387
{
367-
result = result.Substring(0, match.Index + match.Length);
368-
var charCount = _encoding.GetByteCount(result);
388+
returnText = matchText.Substring(0, match.Index + match.Length);
389+
var returnLength = _encoding.GetByteCount(returnText);
369390

370391
// Remove processed items from the queue
371-
for (var i = 0; i < charCount && _incoming.Count > 0; i++)
392+
for (var i = 0; i < returnLength && _incoming.Count > 0; i++)
372393
{
394+
if (_expect.Count == _incoming.Count)
395+
{
396+
_ = _expect.Dequeue();
397+
}
398+
373399
_ = _incoming.Dequeue();
374400
}
375401

@@ -390,7 +416,7 @@ public string Expect(Regex regex, TimeSpan timeout)
390416
}
391417
}
392418

393-
return result;
419+
return returnText;
394420
}
395421

396422
/// <summary>
@@ -446,7 +472,8 @@ public IAsyncResult BeginExpect(AsyncCallback callback, object state, params Exp
446472
public IAsyncResult BeginExpect(TimeSpan timeout, AsyncCallback callback, object state, params ExpectAction[] expectActions)
447473
#pragma warning restore CA1859 // Use concrete types when possible for improved performance
448474
{
449-
var text = string.Empty;
475+
var matchText = string.Empty;
476+
string returnText;
450477

451478
// Create new AsyncResult object
452479
var asyncResult = new ExpectAsyncResult(callback, state);
@@ -461,31 +488,36 @@ public IAsyncResult BeginExpect(TimeSpan timeout, AsyncCallback callback, object
461488
{
462489
lock (_incoming)
463490
{
464-
if (_incoming.Count > 0)
491+
if (_expect.Count > 0)
465492
{
466-
text = _encoding.GetString(_incoming.ToArray(), 0, _incoming.Count);
493+
matchText = _encoding.GetString(_expect.ToArray(), 0, _expect.Count);
467494
}
468495

469-
if (text.Length > 0)
496+
if (matchText.Length > 0)
470497
{
471498
foreach (var expectAction in expectActions)
472499
{
473-
var match = expectAction.Expect.Match(text);
500+
var match = expectAction.Expect.Match(matchText);
474501

475502
if (match.Success)
476503
{
477-
var result = text.Substring(0, match.Index + match.Length);
478-
var charCount = _encoding.GetByteCount(result);
504+
returnText = matchText.Substring(0, match.Index + match.Length);
505+
var returnLength = _encoding.GetByteCount(returnText);
479506

480-
for (var i = 0; i < match.Index + match.Length && _incoming.Count > 0; i++)
507+
// Remove processed items from the queue
508+
for (var i = 0; i < returnLength && _incoming.Count > 0; i++)
481509
{
482-
// Remove processed items from the queue
510+
if (_expect.Count == _incoming.Count)
511+
{
512+
_ = _expect.Dequeue();
513+
}
514+
483515
_ = _incoming.Dequeue();
484516
}
485517

486-
expectAction.Action(result);
518+
expectAction.Action(returnText);
487519
callback?.Invoke(asyncResult);
488-
expectActionResult = result;
520+
expectActionResult = returnText;
489521
}
490522
}
491523
}
@@ -584,6 +616,11 @@ public string ReadLine(TimeSpan timeout)
584616
// remove processed bytes from the queue
585617
for (var i = 0; i < bytesProcessed; i++)
586618
{
619+
if (_expect.Count == _incoming.Count)
620+
{
621+
_ = _expect.Dequeue();
622+
}
623+
587624
_ = _incoming.Dequeue();
588625
}
589626

@@ -620,6 +657,7 @@ public string Read()
620657
lock (_incoming)
621658
{
622659
text = _encoding.GetString(_incoming.ToArray(), 0, _incoming.Count);
660+
_expect.Clear();
623661
_incoming.Clear();
624662
}
625663

@@ -649,6 +687,11 @@ public override int Read(byte[] buffer, int offset, int count)
649687
{
650688
for (; i < count && _incoming.Count > 0; i++)
651689
{
690+
if (_expect.Count == _incoming.Count)
691+
{
692+
_ = _expect.Dequeue();
693+
}
694+
652695
buffer[offset + i] = _incoming.Dequeue();
653696
}
654697
}
@@ -800,6 +843,12 @@ private void Channel_DataReceived(object sender, ChannelDataEventArgs e)
800843
foreach (var b in e.Data)
801844
{
802845
_incoming.Enqueue(b);
846+
if (_expect.Count == _expectSize)
847+
{
848+
_ = _expect.Dequeue();
849+
}
850+
851+
_expect.Enqueue(b);
803852
}
804853
}
805854

‎src/Renci.SshNet/SshClient.cs

+67-4
Original file line numberDiff line numberDiff line change
@@ -407,7 +407,7 @@ public Shell CreateShell(Encoding encoding, string input, Stream output, Stream
407407
/// <remarks>
408408
/// <para>
409409
/// The <c>TERM</c> environment variable contains an identifier for the text window's capabilities.
410-
/// You can get a detailed list of these cababilities by using the ‘infocmp’ command.
410+
/// You can get a detailed list of these capabilities by using the ‘infocmp’ command.
411411
/// </para>
412412
/// <para>
413413
/// The column/row dimensions override the pixel dimensions(when nonzero). Pixel dimensions refer
@@ -416,7 +416,38 @@ public Shell CreateShell(Encoding encoding, string input, Stream output, Stream
416416
/// </remarks>
417417
public ShellStream CreateShellStream(string terminalName, uint columns, uint rows, uint width, uint height, int bufferSize)
418418
{
419-
return CreateShellStream(terminalName, columns, rows, width, height, bufferSize, terminalModeValues: null);
419+
return CreateShellStream(terminalName, columns, rows, width, height, bufferSize, bufferSize * 2, terminalModeValues: null);
420+
}
421+
422+
/// <summary>
423+
/// Creates the shell stream.
424+
/// </summary>
425+
/// <param name="terminalName">The <c>TERM</c> environment variable.</param>
426+
/// <param name="columns">The terminal width in columns.</param>
427+
/// <param name="rows">The terminal width in rows.</param>
428+
/// <param name="width">The terminal width in pixels.</param>
429+
/// <param name="height">The terminal height in pixels.</param>
430+
/// <param name="bufferSize">The size of the buffer.</param>
431+
/// <param name="expectSize">The size of the expect buffer.</param>
432+
/// <returns>
433+
/// The created <see cref="ShellStream"/> instance.
434+
/// </returns>
435+
/// <exception cref="SshConnectionException">Client is not connected.</exception>
436+
/// <remarks>
437+
/// <para>
438+
/// The <c>TERM</c> environment variable contains an identifier for the text window's capabilities.
439+
/// You can get a detailed list of these capabilities by using the ‘infocmp’ command.
440+
/// </para>
441+
/// <para>
442+
/// The column/row dimensions override the pixel dimensions(when non-zero). Pixel dimensions refer
443+
/// to the drawable area of the window.
444+
/// </para>
445+
/// </remarks>
446+
public ShellStream CreateShellStream(string terminalName, uint columns, uint rows, uint width, uint height, int bufferSize, int expectSize)
447+
{
448+
EnsureSessionIsOpen();
449+
450+
return CreateShellStream(terminalName, columns, rows, width, height, bufferSize, expectSize, terminalModeValues: null);
420451
}
421452

422453
/// <summary>
@@ -436,7 +467,7 @@ public ShellStream CreateShellStream(string terminalName, uint columns, uint row
436467
/// <remarks>
437468
/// <para>
438469
/// The <c>TERM</c> environment variable contains an identifier for the text window's capabilities.
439-
/// You can get a detailed list of these cababilities by using the ‘infocmp’ command.
470+
/// You can get a detailed list of these capabilities by using the ‘infocmp’ command.
440471
/// </para>
441472
/// <para>
442473
/// The column/row dimensions override the pixel dimensions(when non-zero). Pixel dimensions refer
@@ -447,7 +478,39 @@ public ShellStream CreateShellStream(string terminalName, uint columns, uint row
447478
{
448479
EnsureSessionIsOpen();
449480

450-
return ServiceFactory.CreateShellStream(Session, terminalName, columns, rows, width, height, terminalModeValues, bufferSize);
481+
return CreateShellStream(terminalName, columns, rows, width, height, bufferSize, bufferSize * 2, terminalModeValues);
482+
}
483+
484+
/// <summary>
485+
/// Creates the shell stream.
486+
/// </summary>
487+
/// <param name="terminalName">The <c>TERM</c> environment variable.</param>
488+
/// <param name="columns">The terminal width in columns.</param>
489+
/// <param name="rows">The terminal width in rows.</param>
490+
/// <param name="width">The terminal width in pixels.</param>
491+
/// <param name="height">The terminal height in pixels.</param>
492+
/// <param name="bufferSize">The size of the buffer.</param>
493+
/// <param name="expectSize">The size of the expect buffer.</param>
494+
/// <param name="terminalModeValues">The terminal mode values.</param>
495+
/// <returns>
496+
/// The created <see cref="ShellStream"/> instance.
497+
/// </returns>
498+
/// <exception cref="SshConnectionException">Client is not connected.</exception>
499+
/// <remarks>
500+
/// <para>
501+
/// The <c>TERM</c> environment variable contains an identifier for the text window's capabilities.
502+
/// You can get a detailed list of these capabilities by using the ‘infocmp’ command.
503+
/// </para>
504+
/// <para>
505+
/// The column/row dimensions override the pixel dimensions(when non-zero). Pixel dimensions refer
506+
/// to the drawable area of the window.
507+
/// </para>
508+
/// </remarks>
509+
public ShellStream CreateShellStream(string terminalName, uint columns, uint rows, uint width, uint height, int bufferSize, int expectSize, IDictionary<TerminalModes, uint> terminalModeValues)
510+
{
511+
EnsureSessionIsOpen();
512+
513+
return ServiceFactory.CreateShellStream(Session, terminalName, columns, rows, width, height, terminalModeValues, bufferSize, expectSize);
451514
}
452515

453516
/// <summary>

0 commit comments

Comments
 (0)