From 0f2c57740d1b94b0dbabcdf23e557ef81dff7ed4 Mon Sep 17 00:00:00 2001 From: Tim Smith Date: Thu, 27 Aug 2026 20:56:40 -0700 Subject: [PATCH 1/2] Teach cspell the words these files already use The spellcheck job only scans files a PR touches, so any change to lib/mixlib/shellout/unix.rb or lib/mixlib/shellout.rb fails on identifiers that have been in those files for years: cgroupv, ducktype, endgrent, getgrent, LOGNAME, pgid, secondarygroups, seconderies, sgids and WNOHANG. "proccess" was flagged too, but that one is an actual typo in a comment rather than a word worth teaching the dictionary, so fix it instead. Signed-off-by: Tim Smith --- cspell.json | 10 ++++++++++ lib/mixlib/shellout/unix.rb | 2 +- 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/cspell.json b/cspell.json index d4bbb9d..e52f6db 100644 --- a/cspell.json +++ b/cspell.json @@ -152,6 +152,7 @@ "certstore", "CFPREFERENCES", "cfprefsd", + "cgroupv", "chaput", "chardev", "chatops", @@ -333,6 +334,7 @@ "dscacheutil", "dscresource", "dslocal", + "ducktype", "DUPEUX", "DWORDLONG", "DYNALINK", @@ -358,6 +360,7 @@ "encap", "Encryptor", "encryptor", + "endgrent", "endlocal", "entriesread", "envdata", @@ -451,6 +454,7 @@ "GETFD", "GETFL", "getgr", + "getgrent", "getgrgid", "getgrnam", "gethostbyname", @@ -704,6 +708,7 @@ "loginclass", "loginwindow", "LOGLOCATION", + "LOGNAME", "logopts", "logstring", "LONGLONG", @@ -1017,6 +1022,7 @@ "PFILETIME", "PFLOAT", "PGENERICMAPPING", + "pgid", "phabricator", "PHALF", "PHANDLE", @@ -1245,6 +1251,8 @@ "Scriptable", "SCROLLBAR", "SCROLLBARS", + "secondarygroups", + "seconderies", "secontext", "secoption", "secopts", @@ -1291,6 +1299,7 @@ "SETTINGCHANGE", "setuid", "SETX", + "sgids", "SHARENAME", "SHAs", "shas", @@ -1626,6 +1635,7 @@ "WINVER", "WKSTA", "WMIGUID", + "WNOHANG", "woot", "workdir", "WPARAM", diff --git a/lib/mixlib/shellout/unix.rb b/lib/mixlib/shellout/unix.rb index a2b918b..9f850d9 100644 --- a/lib/mixlib/shellout/unix.rb +++ b/lib/mixlib/shellout/unix.rb @@ -112,7 +112,7 @@ def run_command unless ready_buffers @execution_time += READ_WAIT_TIME if @execution_time >= timeout && !@result - # kill the bad proccess + # kill the bad process reap_errant_child # read anything it wrote when we killed it attempt_buffer_read From 06a52e6e8283696c00965949aaaf922e5266f1d0 Mon Sep 17 00:00:00 2001 From: Tim Smith Date: Thu, 27 Aug 2026 21:00:28 -0700 Subject: [PATCH 2/2] Reuse a scratch buffer when reading child output read_nonblock allocates a fresh READ_SIZE String on every read. For a command with a lot of output that is nearly all of the garbage ShellOut produces -- an 8 MB stdout means ~2000 needless 4 KB allocations. It takes an optional buffer to read into instead, so hand it one and reuse it. That is only safe because #<< copies the bytes into @stdout / @stderr; a live stream is free to hold onto whatever it is handed, so when one is attached each read still gets its own String. That split is what the new #drain helper expresses, and it also collapses the three near-identical read loops into one. cat an 8 MB file, ruby 4.0.6 median min objects/run before 30.76 ms 16.81 ms 1997 after 16.10 ms 13.42 ms 100 with live_stream (unchanged path) before 30.38 ms 19.56 ms 3957 after 31.64 ms 17.32 ms 3960 48% faster on the common path with 95% fewer allocations, and no change for live streams. Output is byte-identical: verified equal SHA256 and encoding for 8 MB of stdout, and for interleaved binary stdout/stderr. Signed-off-by: Tim Smith --- lib/mixlib/shellout/unix.rb | 42 +++++++++++++++++++++++++++---------- 1 file changed, 31 insertions(+), 11 deletions(-) diff --git a/lib/mixlib/shellout/unix.rb b/lib/mixlib/shellout/unix.rb index 9f850d9..a2fc4a2 100644 --- a/lib/mixlib/shellout/unix.rb +++ b/lib/mixlib/shellout/unix.rb @@ -288,30 +288,50 @@ def attempt_buffer_read(timeout = READ_WAIT_TIME) ready end - def read_stdout_to_buffer - while ( chunk = child_stdout.read_nonblock(READ_SIZE) ) - @stdout << chunk - @live_stdout << chunk if @live_stdout + # Drain everything currently readable on +io+ into +buffer+, and into + # +live+ as well when a live stream is attached. + # + # Without a live stream we can hand read_nonblock a scratch buffer to + # read into and reuse it for every read, rather than letting it allocate + # a fresh READ_SIZE String each time -- for an output-heavy command that + # is nearly all of the garbage this class produces. Reuse is safe here + # only because #<< copies the bytes into +buffer+. + # + # A live stream is free to hold onto whatever it is handed, so when one + # is attached each read gets its own String, exactly as before. + def drain(io, buffer, live, scratch) + if live + while ( chunk = io.read_nonblock(READ_SIZE) ) + buffer << chunk + live << chunk + end + else + while ( chunk = io.read_nonblock(READ_SIZE, scratch) ) + buffer << chunk + end end + end + + def read_buffer(name) + (@read_buffers ||= {})[name] ||= String.new + end + + def read_stdout_to_buffer + drain(child_stdout, @stdout, @live_stdout, read_buffer(:stdout)) rescue Errno::EAGAIN rescue EOFError open_pipes.delete(child_stdout) end def read_stderr_to_buffer - while ( chunk = child_stderr.read_nonblock(READ_SIZE) ) - @stderr << chunk - @live_stderr << chunk if @live_stderr - end + drain(child_stderr, @stderr, @live_stderr, read_buffer(:stderr)) rescue Errno::EAGAIN rescue EOFError open_pipes.delete(child_stderr) end def read_process_status_to_buffer - while ( chunk = child_process_status.read_nonblock(READ_SIZE) ) - @process_status << chunk - end + drain(child_process_status, @process_status, nil, read_buffer(:process_status)) rescue Errno::EAGAIN rescue EOFError open_pipes.delete(child_process_status)