Skip to content

Commit 0c5f680

Browse files
stalepwillr3
authored andcommitted
Reduce sh postRun overhead by combining 3 shSync calls into 1
Previously, after each sh command, postRun() made 3 sequential shSync calls: exit code capture, pwd, and exit code restore. Each shSync blocks until prompt detection fires, so this added 3 round-trips of overhead per command. Combine all three into a single command: export __qdup_ec=$?; echo "${__qdup_ec}:::$(pwd)"; (exit $__qdup_ec) This captures exit code, pwd, and restores exit code in one round-trip. The response is parsed using indexOf to handle the unlikely case of ::: appearing in directory names. If parsing fails (e.g. noisy output from concurrent processes), retries re-echo the saved variable. If all retries fail, the run is aborted with a clear error message rather than continuing in an unknown state.
1 parent 2f94fb7 commit 0c5f680

1 file changed

Lines changed: 55 additions & 20 deletions

File tree

  • qDup-core/src/main/java/io/hyperfoil/tools/qdup/cmd/impl

qDup-core/src/main/java/io/hyperfoil/tools/qdup/cmd/impl/Sh.java

Lines changed: 55 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -126,42 +126,77 @@ public void postRun(String output,Context context){
126126
context.getShell().removeLineObserver("stream");
127127
}
128128

129-
//if the remove shell has exit codes and the response came from the base shell
129+
//if the remote shell has exit codes and the response came from the base shell
130130
//TODO the base shell is not the only one that supports exit code checking
131131
if(context.getShell()!=null &&
132132
context.getShell().isOpen() &&
133-
/*SshSession.PROMPT.equals(getPreviousPrompt()) &&*/
134133
context.getShell().isPromptShell(getPreviousPrompt()) &&
135134
context.getShell().getHost().isShell())
136135
{
136+
// Combined exit code capture + pwd + exit code restore in a single shSync
137+
// to reduce per-command overhead from 3 round-trips to 1
138+
String combined = context.getShell().shSync("export __qdup_ec=$?; echo \"${__qdup_ec}:::$(pwd)\"; (exit $__qdup_ec)");
137139

138-
String response = context.getShell().shSync("export __qdup_ec=$?; echo $__qdup_ec;");
139-
// ensure the output does not contain characters from other processes
140-
// this gets into a hot loop when ctrl+C the process
140+
String response = "";
141+
String pwd = "";
142+
boolean parsed = false;
143+
144+
// Parse combined response: "exitCode:::pwd" using indexOf to avoid
145+
// issues if ::: appears in the pwd path
146+
if (combined != null && !combined.isBlank()) {
147+
String trimmed = combined.trim();
148+
int delimIndex = trimmed.indexOf(":::");
149+
if (delimIndex > 0) {
150+
String exitPart = trimmed.substring(0, delimIndex).trim();
151+
String pwdPart = trimmed.substring(delimIndex + 3).trim();
152+
if (exitPart.matches("-?\\d+")) {
153+
response = exitPart;
154+
pwd = pwdPart;
155+
parsed = true;
156+
}
157+
}
158+
}
159+
160+
// Fallback: if parsing failed due to noisy output from concurrent processes,
161+
// retry by re-echoing the saved variable
141162
int retry = 0;
142-
while(retry < 5 && !response.matches("-?\\d+") && !response.isBlank() && context.getShell().isReady() && !context.isAborted()){
143-
response = context.getShell().shSync("echo $__qdup_ec;");
163+
while (!parsed && retry < 5 && combined != null && !combined.isBlank() && context.getShell().isReady() && !context.isAborted()) {
164+
combined = context.getShell().shSync("echo \"${__qdup_ec}:::$(pwd)\"");
165+
if (combined != null) {
166+
String trimmed = combined.trim();
167+
int delimIndex = trimmed.indexOf(":::");
168+
if (delimIndex > 0) {
169+
String exitPart = trimmed.substring(0, delimIndex).trim();
170+
String pwdPart = trimmed.substring(delimIndex + 3).trim();
171+
if (exitPart.matches("-?\\d+")) {
172+
response = exitPart;
173+
pwd = pwdPart;
174+
parsed = true;
175+
}
176+
}
177+
}
144178
retry++;
145179
}
146-
if (!response.isBlank() && context.getShell().isReady() && !context.isAborted()) {
147-
//trying to only run these if the previous shSync had a result to avoid deadlocking
148-
String pwd = context.getShell().shSync("pwd");
180+
// If we had to retry, $? was changed by the retries, so restore it
181+
if (retry > 0 && parsed) {
182+
context.getShell().shSync("(exit $__qdup_ec)");
183+
}
184+
185+
if (!parsed) {
186+
// All retries failed — log error and abort to avoid running in an unknown state
187+
context.error("failed to parse exit code and pwd from postRun output [" + combined + "] for " + this);
188+
context.abort(false);
189+
} else if (context.getShell().isReady() && !context.isAborted()) {
149190
context.setCwd(pwd);
150191
if(response.matches("\\d+")){
151192
try {
152193
context.getCommandTimer().getData().set("exit_code", Integer.parseInt(response));
153194
}catch (NumberFormatException e){
154-
//this really shouldn't happen but an un-caught exception would be catastrophic
155195
context.getCommandTimer().getData().set("exit_code", response);
156196
}
157197
} else {
158-
//well this would be a surprise but craziness can happen
159198
context.getCommandTimer().getData().set("exit_code", response);
160199
}
161-
//not including this in the profile data atm
162-
//context.getCommandTimer().getData().set("cwd", pwd);
163-
//restore the exit code for any user exit code checking in their script
164-
context.getShell().shSync("(exit $__qdup_ec);");
165200
context.getShell().flushAndResetBuffer();
166201
}
167202
//log the output if not using stream logging
@@ -176,7 +211,7 @@ public void postRun(String output,Context context){
176211
}
177212
}
178213
//abort on non-zero exit if needed
179-
if(!"0".equals(response) && shouldCheckExit(context)){
214+
if(parsed && !"0".equals(response) && shouldCheckExit(context)){
180215
boolean couldBeCtrlC = walk(CmdLocation.createTmp(), (cmd) -> {
181216
return cmd instanceof CtrlC;
182217
}).stream().anyMatch(Boolean::booleanValue);
@@ -197,16 +232,16 @@ public void postRun(String output,Context context){
197232
context.error("aborting run due to unexpected characters in exit code ["+response+"] The prompt may have changed\n host: "+context.getShell().getHost()+"\n command: "+ this +(stack.length()>0?"\nstack:"+stack.toString():""));
198233
}
199234

200-
201235
context.abort(false);
202236
}
203237
}
204238
}else{
205-
//duplicate getting toLog to avoid the overhead if not needed
206239
//turn off stream logging if on
207240
if(!context.getCoordinator().getGlobals().getSetting(Globals.STREAM_LOGGING,false)){
208241
String toLog = getLogOutput(output,context);
209-
context.log(toLog);
242+
if (toLog != null && !toLog.isBlank()) {
243+
context.log(toLog);
244+
}
210245
}
211246
}
212247
}

0 commit comments

Comments
 (0)