Skip to content

Conversation

@not-matthias
Copy link
Member

@not-matthias not-matthias commented Jan 20, 2026

  • Running the perf ping everytime before we receive a command puts unnecessary load on the other end, and also has the chance of failing to receive an answer in time.
  • Also added a test to ensure the FIFO is cancel-safe. This can be an issue when we read the len but then timeout and drop the future, since read_exact isn't cancel-safe. So we'll never handle this command and the integration never receives a response
  • Instead of using the perf fifo to check if the process is still alive, we now check if all the sub-processes terminates. The bash process will still exist since it will terminate on process.wait() after the FIFO handler terminates (an unfortunate limitation of having a single-threaded async runtime)

@codspeed-hq
Copy link

codspeed-hq bot commented Jan 20, 2026

Merging this PR will not alter performance

✅ 4 untouched benchmarks


Comparing cod-2057-perf-timeout-in-codspeed-go-makes-benchmark-hang (9035594) with main (06d99f2)

Open in CodSpeed

@not-matthias not-matthias force-pushed the cod-2057-perf-timeout-in-codspeed-go-makes-benchmark-hang branch 4 times, most recently from de60360 to 9db73b3 Compare January 21, 2026 17:56
@not-matthias not-matthias force-pushed the cod-2057-perf-timeout-in-codspeed-go-makes-benchmark-hang branch from 9db73b3 to f68eabe Compare January 21, 2026 18:00
Running the perf ping everytime before we receive a command puts unnecessary load on the other end, and also has the chance of failing to receive an answer in time.
The FIFO check is very error prone and also slows down the teardown, since we're waiting for the last timeout.
@not-matthias not-matthias force-pushed the cod-2057-perf-timeout-in-codspeed-go-makes-benchmark-hang branch from bd6f4bc to 9035594 Compare January 22, 2026 12:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants