From 3b5d778d4d95b88a73185695da3cb7cfcc51421c Mon Sep 17 00:00:00 2001 From: apnadkarni Date: Mon, 19 Feb 2024 16:58:48 +0000 Subject: Starton [bda99f2393]. --- win/tclWinConsole.c | 45 ++++++++++++++++++++++++++++++--------------- 1 file changed, 30 insertions(+), 15 deletions(-) diff --git a/win/tclWinConsole.c b/win/tclWinConsole.c index eb81370..811577d 100644 --- a/win/tclWinConsole.c +++ b/win/tclWinConsole.c @@ -1630,9 +1630,16 @@ ConsoleReaderThread( { ConsoleHandleInfo *handleInfoPtr = (ConsoleHandleInfo *) arg; ConsoleHandleInfo **iterator; - char inputChars[200]; /* Temporary buffer */ Tcl_Size inputLen = 0; Tcl_Size inputOffset = 0; + Tcl_Size lastReadSize = 0; + DWORD sleepTime; + /* + * ReadConsole will limit input to the greater of 256 characters + * and the size of the input buffer. 8.6 used 8192 (4096 chars) + * and so do we. + */ + char inputChars[8192]; /* * Keep looping until one of the following happens. @@ -1666,7 +1673,6 @@ ConsoleReaderThread( Tcl_Size nStored; assert((inputLen - inputOffset) > 0); - nStored = RingBufferIn(&handleInfoPtr->buffer, inputOffset + inputChars, inputLen - inputOffset, @@ -1713,21 +1719,27 @@ ConsoleReaderThread( continue; } + assert(inputLen == 0); + /* - * Both shared buffer and private buffer are empty. Need to go get - * data from console but do not want to read ahead because the - * interp thread might change the read mode, e.g. turning off echo - * for password input. So only do so if at least one interpreter has - * requested data. + * Read more data in two cases: + * 1. The previous read filled the buffer and there could be more + * data in the console internal *text* buffer. Note + * ConsolePendingInput (checked in ConsoleDataAvailable) will NOT + * show this. It holds input events not yet translated to text. + * 2. Tcl threads want more data AND there is data in the + * ConsolePendingInput buffer. The latter check necessary because + * we do not want to read ahead because the interp thread might + * change the read mode, e.g. turning off echo for password + * input. So only do so if at least one interpreter has requested + * data. */ - if ((handleInfoPtr->flags & CONSOLE_DATA_AWAITED) - && ConsoleDataAvailable(handleInfoPtr->console)) { + if (lastReadSize == sizeof(inputChars) + || ((handleInfoPtr->flags & CONSOLE_DATA_AWAITED) + && ConsoleDataAvailable(handleInfoPtr->console))) { DWORD error; /* Do not hold the lock while blocked in console */ ReleaseSRWLockExclusive(&handleInfoPtr->lock); - /* - * Note - the temporary buffer serves two purposes. It - */ error = ReadConsoleChars(handleInfoPtr->console, (WCHAR *)inputChars, sizeof(inputChars) / sizeof(WCHAR), @@ -1735,17 +1747,21 @@ ConsoleReaderThread( AcquireSRWLockExclusive(&handleInfoPtr->lock); if (error == 0) { inputLen *= sizeof(WCHAR); - } else { + lastReadSize = inputLen; + } + else { /* * We only store the last error. It is up to channel * handlers whether to close or not in case of errors. */ + lastReadSize = 0; handleInfoPtr->lastError = error; if (handleInfoPtr->lastError == ERROR_INVALID_HANDLE) { handleInfoPtr->console = INVALID_HANDLE_VALUE; } } - } else { + } + else { /* * Either no one was asking for data, or no data was available. * In the former case, wait until someone wakes us asking for @@ -1753,7 +1769,6 @@ ConsoleReaderThread( * poll since ReadConsole does not support async operation. * So sleep for a short while and loop back to retry. */ - DWORD sleepTime; sleepTime = handleInfoPtr->flags & CONSOLE_DATA_AWAITED ? 50 : INFINITE; SleepConditionVariableSRW(&handleInfoPtr->consoleThreadCV, -- cgit v0.12 From bd822cee645a36744cdbb369c2d4466cd8ba61ad Mon Sep 17 00:00:00 2001 From: apnadkarni Date: Tue, 20 Feb 2024 05:49:14 +0000 Subject: Bump blocking read buffer size to 8192 irrespective of Tcl channel buffer size --- win/tclWinConsole.c | 41 ++++++++++++++++++++++++++--------------- 1 file changed, 26 insertions(+), 15 deletions(-) diff --git a/win/tclWinConsole.c b/win/tclWinConsole.c index 811577d..acd5851 100644 --- a/win/tclWinConsole.c +++ b/win/tclWinConsole.c @@ -70,14 +70,23 @@ static int gInitialized = 0; /* - * Permit CONSOLE_BUFFER_SIZE to be defined on build command for stress test. - * + * INPUT_BUFFER_SIZE is size of buffer passed to ReadConsole in bytes. + * Note that ReadConsole will only allow reading of line lengths up to the + * max of 256 and buffer size passed to it. So dropping this below 512 + * means user can type at most 256 chars. + */ +#ifndef INPUT_BUFFER_SIZE +#define INPUT_BUFFER_SIZE 8192 /* In bytes, so 4096 chars */ +#endif + +/* + * CONSOLE_BUFFER_SIZE is size of storage used in ring buffers. * In theory, at least sizeof(WCHAR) but note the Tcl channel bug * https://core.tcl-lang.org/tcl/tktview/b3977d199b08e3979a8da970553d5209b3042e9c * will cause failures in test suite if close to max input line in the suite. */ #ifndef CONSOLE_BUFFER_SIZE -#define CONSOLE_BUFFER_SIZE 8000 /* In bytes */ +#define CONSOLE_BUFFER_SIZE 8192 /* In bytes */ #endif /* @@ -1143,15 +1152,22 @@ ConsoleInputProc( /* * Blocking read. Just get data from directly from console. There - * is a small complication in that we can only read even number - * of bytes (wide-character API) and the destination buffer should be - * WCHAR aligned. If either condition is not met, we defer to the - * reader thread which handles these case rather than dealing with + * is a small complication in that + * 1. The destination buffer should be WCHAR aligned. + * 2. We can only read even number of bytes (wide-character API). + * 3. Caller has large enough buffer (else length of line user can + * enter will be limited) + * If any condition is not met, we defer to the + * reader thread which handles these cases rather than dealing with * them here (which is a little trickier than it might sound.) + * + * TODO - not clear this block is a useful optimization. bufSize by + * default is 4K which is < INPUT_BUFFER_SIZE and will rarely be + * increased on stdin. */ if ((1 & (size_t)bufPtr) == 0 /* aligned buffer */ - && bufSize > 1 /* Not single byte read */ - ) { + && (1 & bufSize) == 0 /* Even number of bytes */ + && bufSize > INPUT_BUFFER_SIZE) { DWORD lastError; Tcl_Size numChars; ReleaseSRWLockExclusive(&handleInfoPtr->lock); @@ -1634,12 +1650,7 @@ ConsoleReaderThread( Tcl_Size inputOffset = 0; Tcl_Size lastReadSize = 0; DWORD sleepTime; - /* - * ReadConsole will limit input to the greater of 256 characters - * and the size of the input buffer. 8.6 used 8192 (4096 chars) - * and so do we. - */ - char inputChars[8192]; + char inputChars[INPUT_BUFFER_SIZE]; /* * Keep looping until one of the following happens. -- cgit v0.12