From 72604e4fabddbb2a9f8ba2bd6497529600f65028 Mon Sep 17 00:00:00 2001 From: Ellian Carlos Date: Thu, 24 Sep 2026 21:54:21 +0000 Subject: [PATCH 01/25] perf(blobExec): Reuse tokenizer workers Keep one protocol-speaking tokenizer process per walker slot so each blob no longer spawns the timeout and wrapper chain. Replace wedged or dead workers while preserving BlobExec's existing outcome semantics. (cherry picked from commit 295c1d40cf892c06cecc749e27ad2e2dee973650) --- .../main/scala/cregit/blobexec/BlobExec.scala | 5 +- .../src/main/scala/cregit/blobexec/Main.scala | 55 +++- .../cregit/blobexec/TokenizerWorkerPool.scala | 282 ++++++++++++++++++ .../main/scala/cregit/blobexec/Walker.scala | 6 +- .../blobexec/TokenizerWorkerPoolSpec.scala | 177 +++++++++++ run_pipeline_process.sh | 11 + 6 files changed, 530 insertions(+), 6 deletions(-) create mode 100644 blobExec/src/main/scala/cregit/blobexec/TokenizerWorkerPool.scala create mode 100644 blobExec/src/test/scala/cregit/blobexec/TokenizerWorkerPoolSpec.scala diff --git a/blobExec/src/main/scala/cregit/blobexec/BlobExec.scala b/blobExec/src/main/scala/cregit/blobexec/BlobExec.scala index b71e7f2e..f5f8ee33 100644 --- a/blobExec/src/main/scala/cregit/blobexec/BlobExec.scala +++ b/blobExec/src/main/scala/cregit/blobexec/BlobExec.scala @@ -46,11 +46,12 @@ object BlobExec { inserter: ObjectInserter, timeoutSeconds: Int = DefaultTimeoutSeconds, onTimeout: () => Unit = () => (), - onParserCrash: () => Unit = () => () + onParserCrash: () => Unit = () => (), + invocationResult: Option[ChildRunner.Outcome] = None ): Outcome = { val env = Seq("BFG_BLOB" -> origSha, "BFG_FILENAME" -> filename, "BFG_PATH" -> fullPath) - new ChildRunner(timeoutSeconds).run(command, bytes, env) match { + invocationResult.getOrElse(new ChildRunner(timeoutSeconds).run(command, bytes, env)) match { case ChildRunner.Outcome.Killed(why) => System.err.println( s"Warning: command [$command] on blob $origSha at path [$fullPath] gave no usable " + diff --git a/blobExec/src/main/scala/cregit/blobexec/Main.scala b/blobExec/src/main/scala/cregit/blobexec/Main.scala index c608e035..3b45df6b 100644 --- a/blobExec/src/main/scala/cregit/blobexec/Main.scala +++ b/blobExec/src/main/scala/cregit/blobexec/Main.scala @@ -71,10 +71,16 @@ object Main { // `raw` (not `s`): the mask example below contains a regex backslash, which a // processed-escape interpolator rejects. `$$` therefore renders a literal `$`. private val Usage = - raw"""Usage: blobExec [--abort-on-error] [--pipeline | --pipeline-trees | --shard=K/N] [--warm=] [--blob-timeout=] [--stall-timeout=] + raw"""Usage: blobExec [--abort-on-error] [--pipeline | --pipeline-trees | --shard=K/N] [--warm=] [--tokenizer-worker ] [--blob-timeout=] [--stall-timeout=] | | --abort-on-error exit immediately (status 2) on the first non-zero | exit from , instead of skipping that blob + | --tokenizer-worker + | keep one protocol-speaking tokenizer process alive per + | available processor and route blob requests through it. + | The worker receives BFG_MEMO_DIR and BFG_TOKENIZE_CMD + | from this process; without this flag is run + | once per blob as before. | --blob-timeout= | budget for one invocation (default ${BlobExec.DefaultTimeoutSeconds}). | A child that exceeds it is killed with everything @@ -135,7 +141,24 @@ object Main { |""".stripMargin def main(args: Array[String]): Unit = { - val (flags, positional) = args.partition(_.startsWith("-")) + val remainingArgs = Vector.newBuilder[String] + var tokenizerWorkerPath: Option[java.nio.file.Path] = None + var argIndex = 0 + while (argIndex < args.length) { + if (args(argIndex) == "--tokenizer-worker") { + if (tokenizerWorkerPath.isDefined || argIndex + 1 >= args.length) { + System.err.println("Error: --tokenizer-worker requires exactly one path") + System.err.println(Usage) + sys.exit(1) + } + tokenizerWorkerPath = Some(Paths.get(args(argIndex + 1))) + argIndex += 2 + } else { + remainingArgs += args(argIndex) + argIndex += 1 + } + } + val (flags, positional) = remainingArgs.result().partition(_.startsWith("-")) var abortOnError = false var pipeline = false @@ -238,6 +261,12 @@ object Main { System.err.println(s"Error: command [$command] does not exist") sys.exit(1) } + tokenizerWorkerPath.foreach { path => + if (!Files.isRegularFile(path) || !Files.isExecutable(path)) { + System.err.println(s"Error: tokenizer worker [$path] is not an executable file") + sys.exit(1) + } + } if (mask.isEmpty) { System.err.println("Error: fileMaskRegex must be non-empty") sys.exit(1) @@ -258,7 +287,8 @@ object Main { s"abortOnError=$abortOnError pipeline=$pipeline pipelineTrees=$pipelineTrees " + s"shard=$shardStr warm=$warmStr blobTimeout=${blobTimeoutSeconds}s " + s"stallTimeout=${stallTimeoutSeconds}s incremental=$incremental " + - s"denylistEntries=${denylist.size}" + s"denylistEntries=${denylist.size} " + + s"tokenizerWorker=${tokenizerWorkerPath.map(_.toString).getOrElse("off")}" ) val src: FileRepository = openSrc(srcPath) @@ -272,14 +302,28 @@ object Main { // The cumulative figure is forensic only: gating on it would make one past // timeout permanent. + var workerPool: Option[TokenizerWorkerPool] = None + var workerShutdownHook: Option[Thread] = None val (stats, timedOutEver) = try { val parallelism = math.max(1, Runtime.getRuntime.availableProcessors) + workerPool = tokenizerWorkerPath.map { path => + val workerEnv = Seq("BFG_MEMO_DIR", "BFG_TOKENIZE_CMD").flatMap { key => + sys.env.get(key).map(key -> _) + }.toMap + new TokenizerWorkerPool(Seq(path.toString), workerEnv, parallelism, blobTimeoutSeconds) + } + workerShutdownHook = workerPool.map { pool => + val hook = new Thread(() => pool.close(), "tokenizer-worker-pool-shutdown") + Runtime.getRuntime.addShutdownHook(hook) + hook + } val walker = new Walker( src, dst, mapping, mask.r, command, abortOnError, parallelism, pipeline, pipelineTrees, shard, destinationMayContainObjects = incremental, blobTimeoutSeconds = blobTimeoutSeconds, stallTimeoutSeconds = stallTimeoutSeconds, + workerPool = workerPool, denylist = denylist ) val s = walker.run() @@ -288,6 +332,11 @@ object Main { if (s.blobsTimedOut > 0) mapping.setMeta(BlobsTimedOutMetaKey, total.toString) (s, total) } finally { + workerPool.foreach(_.close()) + workerShutdownHook.foreach { hook => + try Runtime.getRuntime.removeShutdownHook(hook) + catch { case _: IllegalStateException => () } + } mapping.close() dst.close() src.close() diff --git a/blobExec/src/main/scala/cregit/blobexec/TokenizerWorkerPool.scala b/blobExec/src/main/scala/cregit/blobexec/TokenizerWorkerPool.scala new file mode 100644 index 00000000..e0171aee --- /dev/null +++ b/blobExec/src/main/scala/cregit/blobexec/TokenizerWorkerPool.scala @@ -0,0 +1,282 @@ +package cregit.blobexec + +import java.io.{BufferedInputStream, BufferedOutputStream, BufferedReader, EOFException, IOException, InputStream, InputStreamReader} +import java.nio.charset.StandardCharsets.UTF_8 +import java.util.concurrent.atomic.{AtomicBoolean, AtomicReference} +import java.util.concurrent.{ArrayBlockingQueue, ConcurrentHashMap, CountDownLatch, TimeUnit} +import scala.jdk.CollectionConverters._ + +final class TokenizerWorkerPool( + workerCommand: Seq[String], + env: Map[String, String], + size: Int, + timeoutSeconds: Int +) extends AutoCloseable { + + require(workerCommand.nonEmpty, "workerCommand must not be empty") + require(size > 0, "worker pool size must be positive") + require(timeoutSeconds > 0, "worker timeout must be positive") + + private val WorkerTimeoutExitCode = 124 + + private final class Response(val exitCode: Int, val stdout: Array[Byte], val stderr: String) + + private final class Worker( + val index: Int, + val process: Process, + val stdin: BufferedOutputStream, + val stdout: BufferedInputStream + ) { + private val diagnostics = new StringBuilder + + def appendDiagnostic(line: String): Unit = diagnostics.synchronized { + diagnostics.append(line).append('\n') + } + + def diagnosticsLength: Int = diagnostics.synchronized(diagnostics.length) + + def diagnosticsSince(offset: Int): String = diagnostics.synchronized { + diagnostics.substring(math.min(offset, diagnostics.length)) + } + } + + private val idle = new ArrayBlockingQueue[Worker](size) + private val workers = ConcurrentHashMap.newKeySet[Worker]() + private val closed = new AtomicBoolean(false) + + try { + (0 until size).foreach { index => + val worker = startWorker(index) + workers.add(worker) + idle.put(worker) + } + } catch { + case failure: Throwable => + close() + throw failure + } + + def invoke( + bytes: Array[Byte], + origSha: String, + filename: String, + fullPath: String + ): ChildRunner.Outcome = { + if (closed.get()) throw new IllegalStateException("tokenizer worker pool is closed") + + val worker = idle.take() + val diagnosticOffset = worker.diagnosticsLength + try { + writeRequest(worker, bytes, origSha, filename, fullPath) + } catch { + case failure: IOException => + return failedWorker(worker, diagnosticOffset, failure) + } + + val finished = new CountDownLatch(1) + val response = new AtomicReference[Either[Throwable, Response]]() + val reader = new Thread( + () => { + try response.set(Right(readResponse(worker.stdout))) + catch { case failure: Throwable => response.set(Left(failure)) } + finally finished.countDown() + }, + s"token-worker-response-${worker.index}" + ) + reader.setDaemon(true) + reader.start() + + val backstopSeconds = math.max(1, timeoutSeconds).toLong + 5L + 5L + if (!finished.await(backstopSeconds, TimeUnit.SECONDS)) { + replaceWorker(worker, force = true) + ChildRunner.Outcome.Killed(s"no response within ${backstopSeconds}s; worker replaced") + } else { + response.get() match { + case Right(result) => + returnWorker(worker) + if (result.exitCode == WorkerTimeoutExitCode) + ChildRunner.Outcome.Killed(s"no exit within ${timeoutSeconds}s") + else ChildRunner.Outcome.Exited(result.exitCode, result.stdout, result.stderr) + case Left(failure) => + failedWorker(worker, diagnosticOffset, failure) + } + } + } + + private[blobexec] def currentWorkerPids: Seq[Long] = + workers.asScala.toVector.map(_.process.pid()) + + override def close(): Unit = { + if (closed.compareAndSet(false, true)) { + val snapshot = workers.asScala.toVector + snapshot.foreach(worker => try worker.stdin.close() catch { case _: IOException => () }) + val deadline = System.nanoTime() + TimeUnit.SECONDS.toNanos(2) + snapshot.foreach { worker => + val remaining = deadline - System.nanoTime() + if (remaining > 0) worker.process.waitFor(remaining, TimeUnit.NANOSECONDS) + } + snapshot.filter(_.process.isAlive).foreach(_.process.destroyForcibly()) + snapshot.foreach(_.process.waitFor(2, TimeUnit.SECONDS)) + workers.clear() + idle.clear() + } + } + + private def startWorker(index: Int): Worker = { + val builder = new ProcessBuilder(workerCommand: _*) + builder.environment().putAll(env.asJava) + val process = builder.start() + val worker = new Worker( + index, + process, + new BufferedOutputStream(process.getOutputStream), + new BufferedInputStream(process.getInputStream) + ) + startStderrForwarder(worker) + + val ready = new AtomicReference[Either[Throwable, String]]() + val readyLatch = new CountDownLatch(1) + val readyReader = new Thread( + () => { + try ready.set(Right(readHeader(worker.stdout))) + catch { case failure: Throwable => ready.set(Left(failure)) } + finally readyLatch.countDown() + }, + s"token-worker-ready-$index" + ) + readyReader.setDaemon(true) + readyReader.start() + + if (!readyLatch.await(30, TimeUnit.SECONDS)) { + process.destroyForcibly() + throw new IllegalStateException(s"tokenizer worker $index did not print READY within 30 seconds") + } + ready.get() match { + case Right("READY") => worker + case Right(line) => + process.destroyForcibly() + throw new IllegalStateException(s"tokenizer worker $index printed [$line] instead of READY") + case Left(failure) => + process.destroyForcibly() + throw new IllegalStateException(s"tokenizer worker $index failed before READY", failure) + } + } + + private def startStderrForwarder(worker: Worker): Unit = { + val thread = new Thread( + () => { + val reader = new BufferedReader(new InputStreamReader(worker.process.getErrorStream, UTF_8)) + try { + Iterator.continually(reader.readLine()).takeWhile(_ != null).foreach { line => + worker.appendDiagnostic(line) + System.err.println(s"tokenWorker[${worker.index}]: $line") + } + } catch { + case failure: IOException if worker.process.isAlive => + val line = s"stderr forwarding failed: ${failure.getMessage}" + worker.appendDiagnostic(line) + System.err.println(s"tokenWorker[${worker.index}]: $line") + case _: IOException => () + } finally try reader.close() catch { case _: IOException => () } + }, + s"token-worker-stderr-${worker.index}" + ) + thread.setDaemon(true) + thread.start() + } + + private def writeRequest( + worker: Worker, + bytes: Array[Byte], + origSha: String, + filename: String, + fullPath: String + ): Unit = { + val filenameBytes = filename.getBytes(UTF_8) + val pathBytes = fullPath.getBytes(UTF_8) + val header = s"REQ $origSha ${bytes.length} ${filenameBytes.length} ${pathBytes.length} $timeoutSeconds\n" + .getBytes(UTF_8) + worker.stdin.write(header) + worker.stdin.write(filenameBytes) + worker.stdin.write(pathBytes) + worker.stdin.write(bytes) + worker.stdin.flush() + } + + private def readResponse(input: InputStream): Response = { + val fields = readHeader(input).split(" ", -1) + if (fields.length != 4 || fields(0) != "RES") + throw new IOException(s"malformed tokenizer worker response header [${fields.mkString(" ")}]") + + val exitCode = parseInt(fields(1), "exit") + val stdoutLength = parseLength(fields(2), "outLen") + val stderrLength = parseLength(fields(3), "errLen") + val stdout = readExactly(input, stdoutLength) + val stderr = new String(readExactly(input, stderrLength), UTF_8) + new Response(exitCode, stdout, stderr) + } + + private def readHeader(input: InputStream): String = { + val bytes = new java.io.ByteArrayOutputStream + var next = input.read() + while (next != -1 && next != '\n') { + if (bytes.size() >= 8192) throw new IOException("tokenizer worker response header exceeds 8192 bytes") + bytes.write(next) + next = input.read() + } + if (next == -1) throw new EOFException("tokenizer worker closed stdout") + new String(bytes.toByteArray, UTF_8) + } + + private def readExactly(input: InputStream, length: Int): Array[Byte] = { + val bytes = new Array[Byte](length) + var offset = 0 + while (offset < length) { + val read = input.read(bytes, offset, length - offset) + if (read == -1) throw new EOFException(s"tokenizer worker closed stdout after $offset of $length bytes") + offset += read + } + bytes + } + + private def parseInt(value: String, name: String): Int = + value.toIntOption.getOrElse(throw new IOException(s"invalid $name [$value] in tokenizer response")) + + private def parseLength(value: String, name: String): Int = { + val length = parseInt(value, name) + if (length < 0) throw new IOException(s"negative $name [$value] in tokenizer response") + length + } + + private def failedWorker( + worker: Worker, + diagnosticOffset: Int, + failure: Throwable + ): ChildRunner.Outcome = { + val workerExit = if (worker.process.waitFor(100, TimeUnit.MILLISECONDS)) Some(worker.process.exitValue()) else None + val diagnostics = worker.diagnosticsSince(diagnosticOffset) + replaceWorker(worker, force = true) + if (workerExit.contains(WorkerTimeoutExitCode)) + ChildRunner.Outcome.Killed(s"worker exited $WorkerTimeoutExitCode (timeout)") + else { + val stderr = if (diagnostics.nonEmpty) diagnostics else Option(failure.getMessage).getOrElse(failure.toString) + ChildRunner.Outcome.Exited(workerExit.filter(_ != 0).getOrElse(1), Array.emptyByteArray, stderr) + } + } + + private def returnWorker(worker: Worker): Unit = { + if (worker.process.isAlive && !closed.get()) idle.put(worker) + else replaceWorker(worker, force = true) + } + + private def replaceWorker(worker: Worker, force: Boolean): Unit = { + workers.remove(worker) + try worker.stdin.close() catch { case _: IOException => () } + if (force && worker.process.isAlive) worker.process.destroyForcibly() + if (!closed.get()) { + val replacement = startWorker(worker.index) + workers.add(replacement) + idle.put(replacement) + } + } +} diff --git a/blobExec/src/main/scala/cregit/blobexec/Walker.scala b/blobExec/src/main/scala/cregit/blobexec/Walker.scala index a9c9f474..0a3211f3 100644 --- a/blobExec/src/main/scala/cregit/blobexec/Walker.scala +++ b/blobExec/src/main/scala/cregit/blobexec/Walker.scala @@ -57,6 +57,7 @@ final class Walker( destinationMayContainObjects: Boolean = true, blobTimeoutSeconds: Int = BlobExec.DefaultTimeoutSeconds, stallTimeoutSeconds: Int = Walker.DefaultStallTimeoutSeconds, + workerPool: Option[TokenizerWorkerPool] = None, denylist: BlobDenylist = BlobDenylist.shipped ) { import Walker._ @@ -842,6 +843,8 @@ final class Walker( try { blobCommandExecutions.increment() val unusable = new AtomicBoolean(false) + val invocationResult = + workerPool.map(_.invoke(bytes, task.origId.name, task.filename, task.fullPath)) val outcome = BlobExec.run( bytes = bytes, origSha = task.origId.name, @@ -852,7 +855,8 @@ final class Walker( inserter = workerInserter, timeoutSeconds = blobTimeoutSeconds, onTimeout = () => { blobsTimedOut.increment(); unusable.set(true) }, - onParserCrash = () => { blobsParserCrashed.increment(); unusable.set(true) } + onParserCrash = () => { blobsParserCrashed.increment(); unusable.set(true) }, + invocationResult = invocationResult ) val res = outcome match { case BlobExec.Outcome.Skip if unusable.get() => diff --git a/blobExec/src/test/scala/cregit/blobexec/TokenizerWorkerPoolSpec.scala b/blobExec/src/test/scala/cregit/blobexec/TokenizerWorkerPoolSpec.scala new file mode 100644 index 00000000..ba0b6a6a --- /dev/null +++ b/blobExec/src/test/scala/cregit/blobexec/TokenizerWorkerPoolSpec.scala @@ -0,0 +1,177 @@ +package cregit.blobexec + +import org.scalatest.BeforeAndAfterAll +import org.scalatest.funsuite.AnyFunSuite +import org.scalatest.matchers.should.Matchers + +import java.nio.charset.StandardCharsets.UTF_8 +import java.nio.file.{Files, Path} +import scala.concurrent.duration._ +import scala.concurrent.{Await, ExecutionContext, Future} + +class TokenizerWorkerPoolSpec extends AnyFunSuite with Matchers with BeforeAndAfterAll { + + import ChildRunner.Outcome.{Exited, Killed} + + private def exited(outcome: ChildRunner.Outcome): (Int, Array[Byte], String) = outcome match { + case Exited(status, stdout, stderr) => (status, stdout, stderr) + case other => fail(s"expected Exited, got $other") + } + + private val tmpDir = Files.createTempDirectory("tokenizer-worker-pool-") + private val worker = tmpDir.resolve("fake-worker.pl") + + override def beforeAll(): Unit = { + Files.writeString( + worker, + """|#!/usr/bin/env perl + |use strict; + |use warnings; + | + |binmode STDIN; + |binmode STDOUT; + |$| = 1; + | + |sub read_exact { + | my ($length) = @_; + | my $value = ''; + | while (length($value) < $length) { + | my $read = read(STDIN, my $chunk, $length - length($value)); + | exit 2 unless defined($read) && $read > 0; + | $value .= $chunk; + | } + | return $value; + |} + | + |print "READY\n"; + |while (defined(my $header = )) { + | chomp $header; + | my ($tag, $sha, $body_len, $fn_len, $path_len, $timeout) = split / /, $header; + | exit 2 unless $tag eq 'REQ'; + | my $filename = read_exact($fn_len); + | my $path = read_exact($path_len); + | my $body = read_exact($body_len); + | + | if ($filename eq 'hang.c') { + | sleep 300 while 1; + | } elsif ($filename eq 'slow.c') { + | sleep 2; + | my $out = uc($body); + | print "RES 0 ", length($out), " 0\n", $out; + | } elsif ($filename eq 'crash.c') { + | print "RES 33 0 5\ncrash"; + | } elsif ($filename eq 'timeout.c') { + | print "RES 124 0 7\ntimeout"; + | } elsif ($filename eq 'die.c') { + | print STDERR "worker died\n"; + | exit 7; + | } else { + | my $out = uc($body); + | print "RES 0 ", length($out), " 0\n", $out; + | } + |} + |""".stripMargin + ) + worker.toFile.setExecutable(true) + } + + override def afterAll(): Unit = deleteRecursive(tmpDir) + + test("round-trips bytes and preserves parser crash status") { + val pool = new TokenizerWorkerPool(Seq(worker.toString), Map.empty, size = 1, timeoutSeconds = 2) + try { + val (exit, stdout, stderr) = exited(pool.invoke( + "hello".getBytes(UTF_8), "a" * 40, "hello.c", "src/hello.c")) + exit shouldEqual 0 + stdout shouldEqual "HELLO".getBytes(UTF_8) + stderr shouldBe empty + + val (crashExit, crashOut, crashErr) = + exited(pool.invoke("bad".getBytes(UTF_8), "b" * 40, "crash.c", "src/crash.c")) + crashExit shouldEqual BlobExec.ParserCrashExitCode + crashOut shouldBe empty + crashErr shouldEqual "crash" + } finally pool.close() + } + + test("times out a wedged worker, replaces it, and serves the next request") { + val pool = new TokenizerWorkerPool(Seq(worker.toString), Map.empty, size = 1, timeoutSeconds = 1) + try { + val started = System.nanoTime() + val hung = pool.invoke("hang".getBytes(UTF_8), "c" * 40, "hang.c", "src/hang.c") + val elapsed = (System.nanoTime() - started).nanos + + hung shouldBe a[Killed] + elapsed should be < 15.seconds + val (nextExit, nextOut, nextErr) = + exited(pool.invoke("next".getBytes(UTF_8), "d" * 40, "next.c", "src/next.c")) + nextExit shouldEqual 0 + nextOut shouldEqual "NEXT".getBytes(UTF_8) + nextErr shouldBe empty + } finally pool.close() + } + + test("serves pool-sized requests in parallel") { + implicit val ec: ExecutionContext = ExecutionContext.global + val pool = new TokenizerWorkerPool(Seq(worker.toString), Map.empty, size = 3, timeoutSeconds = 5) + try { + val started = System.nanoTime() + val requests = (1 to 3).map { n => + Future(exited(pool.invoke(s"body$n".getBytes(UTF_8), f"$n%040x", "slow.c", s"src/slow$n.c"))) + } + val results = Await.result(Future.sequence(requests), 10.seconds) + val elapsed = (System.nanoTime() - started).nanos + + results.map(_._1) should contain only 0 + results.map(r => new String(r._2, UTF_8)).toSet shouldEqual Set("BODY1", "BODY2", "BODY3") + elapsed should be < 4.seconds + } finally pool.close() + } + + test("maps worker timeout status and respawns after mid-request death") { + val pool = new TokenizerWorkerPool(Seq(worker.toString), Map.empty, size = 1, timeoutSeconds = 2) + try { + val timedOut = pool.invoke("slow".getBytes(UTF_8), "e" * 40, "timeout.c", "src/timeout.c") + timedOut shouldBe a[Killed] + + val died = exited(pool.invoke("die".getBytes(UTF_8), "f" * 40, "die.c", "src/die.c")) + died._1 should not equal 0 + died._2 shouldBe empty + died._3 should include("worker died") + + val recovered = exited(pool.invoke("ok".getBytes(UTF_8), "1" * 40, "ok.c", "src/ok.c")) + recovered._1 shouldEqual 0 + recovered._2 shouldEqual "OK".getBytes(UTF_8) + } finally pool.close() + } + + test("close terminates every worker process") { + val pool = new TokenizerWorkerPool(Seq(worker.toString), Map.empty, size = 2, timeoutSeconds = 2) + val pids = pool.currentWorkerPids + pids should have size 2 + + pool.close() + + eventuallyStopped(pids) shouldBe true + } + + private def eventuallyStopped(pids: Seq[Long]): Boolean = { + val deadline = System.nanoTime() + 5.seconds.toNanos + while (System.nanoTime() < deadline && pids.exists(isAlive)) Thread.sleep(25) + pids.forall(pid => !isAlive(pid)) + } + + private def isAlive(pid: Long): Boolean = { + val handle = ProcessHandle.of(pid) + handle.isPresent && handle.get().isAlive + } + + private def deleteRecursive(path: Path): Unit = { + if (Files.isDirectory(path)) { + val entries = Files.list(path) + try entries.forEach(deleteRecursive) finally entries.close() + } + Files.deleteIfExists(path) + () + } +} diff --git a/run_pipeline_process.sh b/run_pipeline_process.sh index b7ed4ed0..989c3f9a 100755 --- a/run_pipeline_process.sh +++ b/run_pipeline_process.sh @@ -82,6 +82,10 @@ Output: with millions of loose objects. A failed pack never aborts the run: the Parquet dataset is the product, and packing only makes later steps faster. + --tokenizer-worker + keep one tokenizeByBlobId/tokenWorker.pl process alive per + blobExec worker slot instead of spawning the full wrapper + chain for every blob (default: off). --memory-limit SIZE cap the DuckDB heap in the dataset generator (step 10). Omit to accept that script's own default of 8GB. Takes an @@ -198,6 +202,7 @@ WORK="../cregit-files" MEMO_DIR="" SKIP_HTML=0 GC_MODE="plain" +TOKENIZER_WORKER=0 MEMORY_LIMIT="" DUCKDB_THREADS="" # Empty means "do not pass the flag", as above: step 10 then emits the @@ -367,6 +372,7 @@ while [ $# -gt 0 ]; do --blob-timeout) need_val "$@"; BLOB_TIMEOUT="$2"; shift 2 ;; --stall-timeout) need_val "$@"; STALL_TIMEOUT="$2"; shift 2 ;; --gc) need_val "$@"; GC_MODE="$2"; shift 2 ;; + --tokenizer-worker) TOKENIZER_WORKER=1; shift ;; --memory-limit) need_val "$@"; MEMORY_LIMIT="$2"; shift 2 ;; --duckdb-threads) need_val "$@"; DUCKDB_THREADS="$2"; shift 2 ;; --project-meta) need_val "$@"; PROJECT_META="$2"; shift 2 ;; @@ -743,7 +749,12 @@ else MODE_FLAG="" [ "$MODE" = "pipeline" ] && MODE_FLAG="--pipeline" [ "$MODE" = "pipeline-trees" ] && MODE_FLAG="--pipeline-trees" + TOKENIZER_WORKER_FLAGS=() + if [ "$TOKENIZER_WORKER" = 1 ]; then + TOKENIZER_WORKER_FLAGS=("--tokenizer-worker" "${CREGIT}/tokenizeByBlobId/tokenWorker.pl") + fi java -jar "$BFG" $MODE_FLAG \ + ${TOKENIZER_WORKER_FLAGS[@]+"${TOKENIZER_WORKER_FLAGS[@]}"} \ ${BLOB_TIMEOUT:+--blob-timeout=$BLOB_TIMEOUT} \ ${STALL_TIMEOUT:+--stall-timeout=$STALL_TIMEOUT} \ "$REPO_PATH_ORIGINAL_BARE" \ From 77e332f990856a3a9a1c6e7e8c97e7735c94866f Mon Sep 17 00:00:00 2001 From: Ellian Carlos Date: Fri, 25 Sep 2026 00:47:11 +0000 Subject: [PATCH 02/25] perf(tokenizeByBlobId): Add persistent tokenizer worker tokenWorker.pl speaks the framed stdin/stdout protocol in WORKER_PROTOCOL.md and replaces the timeout/sh/tokenize.pl/tokenBySha.pl chain per blob with one long-lived process per pool slot. Output and memo bytes are identical to tokenBySha.pl (tests/test_token_worker.sh checks byte equality, memo hits, non-fatal unknown extensions, and 124 on parser timeout with no child left). Body reads use buffered read() rather than sysread(): the header is read with , which fills perl's buffer past the newline, and sysread bypasses that buffer and blocks forever. (cherry picked from commit 11a08483fa928f5f5eed03fdbae30b8edfbe8f09) --- tests/fixtures/quote'name.c | 1 + tests/fixtures/tiny.c | 1 + tests/test_token_worker.sh | 33 ++++ tests/token_worker_driver.py | 196 ++++++++++++++++++++++++ tokenizeByBlobId/WORKER_PROTOCOL.md | 70 +++++++++ tokenizeByBlobId/tokenWorker.pl | 228 ++++++++++++++++++++++++++++ 6 files changed, 529 insertions(+) create mode 100644 tests/fixtures/quote'name.c create mode 100644 tests/fixtures/tiny.c create mode 100755 tests/test_token_worker.sh create mode 100755 tests/token_worker_driver.py create mode 100644 tokenizeByBlobId/WORKER_PROTOCOL.md create mode 100755 tokenizeByBlobId/tokenWorker.pl diff --git a/tests/fixtures/quote'name.c b/tests/fixtures/quote'name.c new file mode 100644 index 00000000..041c166d --- /dev/null +++ b/tests/fixtures/quote'name.c @@ -0,0 +1 @@ +static int quoted_name(void) { return 7; } diff --git a/tests/fixtures/tiny.c b/tests/fixtures/tiny.c new file mode 100644 index 00000000..78f2de10 --- /dev/null +++ b/tests/fixtures/tiny.c @@ -0,0 +1 @@ +int main(void) { return 0; } diff --git a/tests/test_token_worker.sh b/tests/test_token_worker.sh new file mode 100755 index 00000000..79bfdbf7 --- /dev/null +++ b/tests/test_token_worker.sh @@ -0,0 +1,33 @@ +#!/usr/bin/env bash + +set -euo pipefail + +ROOT=$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd) +export PATH="/home/ellianco/Projects/cregit-workspace/cregit-issue61/.devenv/profile/bin:$PATH" + +SRCML2TOKEN="$ROOT/tokenize/srcMLtoken/srcml2token" +if [[ ! -x "$SRCML2TOKEN" ]]; then + if ! pkg-config --exists xerces-c; then + for candidate in /nix/store/*-xerces-c-*/lib/pkgconfig/xerces-c.pc; do + if [[ -f "$candidate" ]]; then + export PKG_CONFIG_PATH="$(dirname "$candidate")${PKG_CONFIG_PATH:+:$PKG_CONFIG_PATH}" + break + fi + done + fi + make -C "$ROOT/tokenize/srcMLtoken" +fi + +SRCML=$(command -v srcml) +CTAGS=$(command -v ctags) +TOKENIZE_COMMAND="$ROOT/tokenize/tokenize.pl --srcml2token=$SRCML2TOKEN --srcml=$SRCML --ctags=$CTAGS" +TEMP_ROOT=$(mktemp -d "${TMPDIR:-/tmp}/cregit-token-worker.XXXXXX") +trap 'rm -rf "$TEMP_ROOT"' EXIT + +python3 "$ROOT/tests/token_worker_driver.py" \ + --worker "$ROOT/tokenizeByBlobId/tokenWorker.pl" \ + --token-by-sha "$ROOT/tokenizeByBlobId/tokenBySha.pl" \ + --tokenize-command "$TOKENIZE_COMMAND" \ + --fixture "$ROOT/tests/fixtures/tiny.c" \ + --fixture "$ROOT/tests/fixtures/quote'name.c" \ + --temp-root "$TEMP_ROOT" diff --git a/tests/token_worker_driver.py b/tests/token_worker_driver.py new file mode 100755 index 00000000..1e4b8cb5 --- /dev/null +++ b/tests/token_worker_driver.py @@ -0,0 +1,196 @@ +#!/usr/bin/env python3 + +import argparse +import hashlib +import os +import pathlib +import subprocess +import tempfile +import time + + +class Worker: + def __init__(self, executable, memo_dir, tokenize_command): + env = os.environ.copy() + env["BFG_MEMO_DIR"] = str(memo_dir) + env["BFG_TOKENIZE_CMD"] = tokenize_command + self.process = subprocess.Popen( + [str(executable)], + stdin=subprocess.PIPE, + stdout=subprocess.PIPE, + stderr=subprocess.PIPE, + env=env, + ) + ready = self.process.stdout.readline() + if ready != b"READY\n": + raise AssertionError(f"worker startup failed: {ready!r}") + + def request(self, filename, full_path, contents, timeout_secs): + filename_bytes = os.fsencode(filename) + path_bytes = os.fsencode(full_path) + orig_sha = hashlib.sha1(contents).hexdigest() + header = ( + f"REQ {orig_sha} {len(contents)} {len(filename_bytes)} " + f"{len(path_bytes)} {timeout_secs}\n" + ).encode("ascii") + self.process.stdin.write(header + filename_bytes + path_bytes + contents) + self.process.stdin.flush() + + response = self.process.stdout.readline() + parts = response.decode("ascii").rstrip("\n").split(" ") + if len(parts) != 4 or parts[0] != "RES": + raise AssertionError(f"invalid response header: {response!r}") + exit_code, out_len, err_len = map(int, parts[1:]) + stdout = self._read_exact(out_len) + stderr = self._read_exact(err_len) + return exit_code, stdout, stderr + + def close(self): + self.process.stdin.close() + exit_code = self.process.wait(timeout=5) + stderr = self.process.stderr.read() + if exit_code != 0: + raise AssertionError( + f"worker exited {exit_code} on EOF: {stderr.decode(errors='replace')}" + ) + + def _read_exact(self, length): + value = b"" + while len(value) < length: + chunk = self.process.stdout.read(length - len(value)) + if not chunk: + raise AssertionError("unexpected EOF from worker") + value += chunk + return value + + +def reference(token_by_sha, memo_dir, tokenize_command, fixture): + contents = fixture.read_bytes() + env = os.environ.copy() + env.update( + { + "BFG_BLOB": hashlib.sha1(contents).hexdigest(), + "BFG_FILENAME": fixture.name, + "BFG_PATH": f"fixtures/{fixture.name}", + "BFG_MEMO_DIR": str(memo_dir), + "BFG_TOKENIZE_CMD": tokenize_command, + } + ) + return subprocess.run( + [str(token_by_sha)], + input=contents, + stdout=subprocess.PIPE, + stderr=subprocess.PIPE, + env=env, + check=False, + ) + + +def memo_file(memo_dir, contents): + digest = hashlib.sha1(contents).hexdigest() + return memo_dir / digest[:2] / digest[2:4] / digest + + +def assert_no_timeout_process(marker): + time.sleep(0.2) + result = subprocess.run( + ["pgrep", "-f", marker], + stdout=subprocess.PIPE, + stderr=subprocess.PIPE, + check=False, + ) + if result.returncode != 1: + raise AssertionError( + f"timeout process still running: {result.stdout.decode().strip()}" + ) + + +def main(): + parser = argparse.ArgumentParser() + parser.add_argument("--worker", required=True, type=pathlib.Path) + parser.add_argument("--token-by-sha", required=True, type=pathlib.Path) + parser.add_argument("--tokenize-command", required=True) + parser.add_argument("--fixture", action="append", required=True, type=pathlib.Path) + parser.add_argument("--temp-root", required=True, type=pathlib.Path) + args = parser.parse_args() + + worker_memo = args.temp_root / "worker-memo" + worker_memo.mkdir() + worker = Worker(args.worker, worker_memo, args.tokenize_command) + + expected_outputs = [] + for index, fixture in enumerate(args.fixture): + reference_memo = args.temp_root / f"reference-memo-{index}" + reference_memo.mkdir() + expected = reference( + args.token_by_sha, reference_memo, args.tokenize_command, fixture + ) + if expected.returncode != 0: + raise AssertionError( + f"reference failed for {fixture}: " + f"{expected.stderr.decode(errors='replace')}" + ) + actual = worker.request( + fixture.name, + f"fixtures/{fixture.name}", + fixture.read_bytes(), + 20, + ) + if actual[0] != 0 or actual[1] != expected.stdout: + raise AssertionError(f"worker output differs for {fixture}") + if memo_file(worker_memo, fixture.read_bytes()).read_bytes() != expected.stdout: + raise AssertionError(f"worker memo differs for {fixture}") + expected_outputs.append(expected.stdout) + print(f"PASS byte-identical: {fixture.name}") + + repeated = worker.request( + args.fixture[0].name, + f"fixtures/{args.fixture[0].name}", + args.fixture[0].read_bytes(), + 20, + ) + if repeated != (0, expected_outputs[0], b""): + raise AssertionError("memo-hit response changed bytes") + print("PASS repeated request uses identical memo bytes") + + unknown = worker.request("unknown.xyzzy", "fixtures/unknown.xyzzy", b"x", 20) + if unknown[0] == 0 or b"unknown file extension" not in unknown[2]: + raise AssertionError(f"unknown extension was not rejected: {unknown!r}") + recovered = worker.request( + args.fixture[0].name, + f"fixtures/{args.fixture[0].name}", + args.fixture[0].read_bytes(), + 20, + ) + if recovered[0] != 0 or recovered[1] != expected_outputs[0]: + raise AssertionError("worker did not recover after unknown extension") + print("PASS unknown extension is non-fatal to worker") + worker.close() + + timeout_script = args.temp_root / "timeout-parser.sh" + marker = f"cregit-token-worker-timeout-{os.getpid()}" + timeout_script.write_text( + "#!/usr/bin/env bash\n" + f"exec -a {marker} sleep 100\n", + encoding="utf-8", + ) + timeout_script.chmod(0o755) + timeout_memo = args.temp_root / "timeout-memo" + timeout_memo.mkdir() + timeout_worker = Worker(args.worker, timeout_memo, str(timeout_script)) + started = time.monotonic() + timed_out = timeout_worker.request("timeout.c", "fixtures/timeout.c", b"int x;\n", 1) + elapsed = time.monotonic() - started + if timed_out[0] != 124 or elapsed > 3.0: + raise AssertionError(f"timeout response was {timed_out[0]} after {elapsed:.2f}s") + if any(timeout_memo.rglob("*")): + raise AssertionError("timeout response was memoized") + assert_no_timeout_process(marker) + timeout_worker.close() + print(f"PASS timeout exits 124 in {elapsed:.2f}s with no child left") + + print("ALL TOKEN WORKER TESTS PASSED") + + +if __name__ == "__main__": + main() diff --git a/tokenizeByBlobId/WORKER_PROTOCOL.md b/tokenizeByBlobId/WORKER_PROTOCOL.md new file mode 100644 index 00000000..267e0149 --- /dev/null +++ b/tokenizeByBlobId/WORKER_PROTOCOL.md @@ -0,0 +1,70 @@ +# Persistent tokenizer worker protocol + +`tokenizeByBlobId/tokenWorker.pl` replaces the per-blob chain +`timeout → sh → tokenize.pl → tokenBySha.pl → tokenizeSrcMl.pl` with one +long-lived perl process per pool slot. blobExec starts `parallelism` workers and +talks to each over its stdin/stdout. One request in flight per worker. + +The output contract is byte-identical to `tokenBySha.pl`: same tokens on +stdout, same memo file written at `$BFG_MEMO_DIR/xx/yy/`, same +exit-code meaning. A tokenized blob's id is a pure function of content, so the +worker must never change bytes. + +## Startup + +``` +tokenWorker.pl # no args; reads BFG_MEMO_DIR and BFG_TOKENIZE_CMD from env +``` + +Worker writes `READY\n` to stdout once, then waits for requests. stderr is for +diagnostics only; blobExec forwards it. + +## Request (JVM → worker stdin) + +``` +REQ \n + +``` + +- `origSha`: hex id of the original blob (was `BFG_BLOB`). +- `filename`: basename with extension (was `BFG_FILENAME`). +- `fullPath`: repo-relative path (was `BFG_PATH`). +- Lengths are decimal byte counts. Strings are raw bytes, no escaping. +- `timeoutSecs`: per-request parser budget the worker must enforce itself. + +## Response (worker stdout → JVM) + +``` +RES \n + +``` + +- `exit`: what `tokenBySha.pl` would have exited with. `0` = tokens follow; + `33` = parser crash (`$PARSER_CRASH_EXIT`); `124` = the worker killed the + parser on timeout; any other non-zero = tokenizer error, stdout empty. +- On a non-zero exit nothing is memoized, exactly as `tokenBySha.pl` does. + +## Timeout + +The worker runs the parser chain in its own process group (`setpgrp` in the +child) and enforces `timeoutSecs` with `alarm`; on expiry it `kill -TERM` then +`-KILL`s the group and answers `RES 124 0 `. blobExec keeps a backstop: +if no `RES` header arrives within `timeoutSecs + grace + slack`, it destroys the +worker, respawns one, and reports the blob as a timeout (`Skip`, never `Replace`). + +## Lifecycle + +- EOF on stdin → worker exits 0. +- A malformed header → worker writes the reason to stderr and exits 2. blobExec + respawns and reports that blob as a Skip. +- Workers hold no cross-request state except the memo dir. Order of requests is + irrelevant. + +## Stages + +- Stage A (this branch): worker does dispatch + sha1 + memo + temp file + in-process and spawns only the language parser (`tokenizeSrcMl.pl`, which in + turn runs ctags and `srcml | srcml2token`). Removes `timeout`, `sh`, + `tokenize.pl`, `tokenBySha.pl` from every blob. +- Stage B (later): fold `tokenizeSrcMl.pl` into the worker as a module so only + ctags, srcml and srcml2token are spawned per blob. diff --git a/tokenizeByBlobId/tokenWorker.pl b/tokenizeByBlobId/tokenWorker.pl new file mode 100755 index 00000000..7ed2fe7d --- /dev/null +++ b/tokenizeByBlobId/tokenWorker.pl @@ -0,0 +1,228 @@ +#!/usr/bin/env perl + +use strict; +use warnings; +use bytes; +use Digest::SHA qw(sha1_hex); +use Errno qw(EINTR); +use File::Copy qw(move); +use File::Path qw(make_path); +use File::Temp qw(tempfile); +use FindBin qw($RealBin); +use IO::Select; +use POSIX qw(_exit); +use lib "$RealBin/../tokenize"; +use CregitLanguages; + +binmode STDIN; +binmode STDOUT; +$| = 1; + +my $memoDir = $ENV{BFG_MEMO_DIR}; +die "You must define BFG_MEMO_DIR\n" if not defined($memoDir) or $memoDir eq ""; +die "Sha dir [$memoDir] does not exist\n" if not -d $memoDir; + +my $tokenizeCmd = $ENV{BFG_TOKENIZE_CMD}; +die "Tokenize command not defined. Use BFG_TOKENIZE_CMD\n" + if not defined($tokenizeCmd) or $tokenizeCmd eq ""; + +my @tokenizeCommand = grep { length($_) } split /\s+/, $tokenizeCmd; +my %mapLang = %CregitLanguages::EXT_LANG; +my $buildDir = "$RealBin/build"; +make_path($buildDir) if not -d $buildDir; + +print STDOUT "READY\n"; + +while (defined(my $header = )) { + if ($header !~ /\AREQ ([[:xdigit:]]+) ([0-9]+) ([0-9]+) ([0-9]+) ([0-9]+)\n\z/) { + protocol_error("malformed request header"); + } + + my ($origSha, $bodyLen, $fnLen, $pathLen, $timeoutSecs) = ($1, $2, $3, $4, $5); + my $filename = read_exact($fnLen, "filename"); + my $fullPath = read_exact($pathLen, "full path"); + my $contents = read_exact($bodyLen, "body"); + + my ($exitCode, $stdout, $stderr); + my $ok = eval { + ($exitCode, $stdout, $stderr) = process_request( + $origSha, $filename, $fullPath, $contents, $timeoutSecs + ); + 1; + }; + if (not $ok) { + $exitCode = 255; + $stdout = ""; + $stderr = $@ || "tokenizer request failed\n"; + } + + print STDOUT "RES $exitCode " . length($stdout) . " " . length($stderr) . "\n"; + print STDOUT $stdout, $stderr; +} + +exit 0; + +sub read_exact { + my ($length, $label) = @_; + my $value = ""; + + while (length($value) < $length) { + my $read = read(STDIN, $value, $length - length($value), length($value)); + protocol_error("unable to read $label: $!") if not defined($read); + protocol_error("unexpected EOF while reading $label") if $read == 0; + } + + return $value; +} + +sub protocol_error { + my ($message) = @_; + print STDERR "$message\n"; + exit 2; +} + +sub process_request { + my ($origSha, $filename, $fullPath, $contents, $timeoutSecs) = @_; + + my $fileExt; + $fileExt = lc($1) if $filename =~ /\.([^.]+)\z/; + if (not defined($fileExt) or not defined($mapLang{$fileExt})) { + my $shownExt = defined($fileExt) ? $fileExt : ""; + return (255, "", "unknown file extension [$shownExt]\n"); + } + + my $sha1 = sha1_hex($contents); + my $dir = "$memoDir/" . substr($sha1, 0, 2) . "/" . substr($sha1, 2, 2); + my $memoFile = "$dir/$sha1"; + + if (-f $memoFile) { + open(my $memo, '<:raw', $memoFile) + or die "unable to open memoized file [$memoFile]: $!\n"; + local $/; + my $output = <$memo>; + close($memo) or die "unable to close memoized file [$memoFile]: $!\n"; + return (0, $output, ""); + } + + my $tempDir = File::Temp->newdir( + "tokdir-XXXXX", + DIR => $buildDir, + CLEANUP => 1, + ); + my $inputName = "input.$fileExt"; + my $inputFile = "$tempDir/$inputName"; + open(my $input, '>:raw', $inputFile) + or die "unable to write temp input [$inputFile]: $!\n"; + print {$input} $contents; + close($input) or die "unable to close temp input [$inputFile]: $!\n"; + + my @command = (@tokenizeCommand, "--language=$mapLang{$fileExt}", $inputName); + my ($exitCode, $output, $error) = run_command( + "$tempDir", $timeoutSecs, @command + ); + return ($exitCode, "", $error) if $exitCode != 0; + + make_path($dir) if not -d $dir; + my ($memoOut, $tempOutput) = tempfile( + "tmpfile-out-XXXXX", + DIR => $buildDir, + UNLINK => 0, + ); + binmode $memoOut; + print {$memoOut} $output; + close($memoOut) or die "unable to close tokenizer output [$tempOutput]: $!\n"; + move($tempOutput, $memoFile) + or die "unable to move tokenizer output to [$memoFile]: $!\n"; + + return (0, $output, $error); +} + +sub run_command { + my ($workingDir, $timeoutSecs, @command) = @_; + pipe(my $stdoutRead, my $stdoutWrite) or die "unable to create stdout pipe: $!\n"; + pipe(my $stderrRead, my $stderrWrite) or die "unable to create stderr pipe: $!\n"; + + my $pid = fork(); + die "unable to fork tokenizer: $!\n" if not defined($pid); + + if ($pid == 0) { + close($stdoutRead); + close($stderrRead); + open(STDOUT, '>&', $stdoutWrite) or _exit(255); + open(STDERR, '>&', $stderrWrite) or _exit(255); + binmode STDOUT; + binmode STDERR; + close($stdoutWrite); + close($stderrWrite); + setpgrp(0, 0) or do { + print STDERR "unable to create tokenizer process group: $!\n"; + _exit(255); + }; + chdir($workingDir) or do { + print STDERR "unable to enter tokenizer directory [$workingDir]: $!\n"; + _exit(255); + }; + exec {$command[0]} @command; + print STDERR "unable to execute tokenizer [$command[0]]: $!\n"; + _exit(255); + } + + close($stdoutWrite); + close($stderrWrite); + binmode $stdoutRead; + binmode $stderrRead; + + my $timedOut = 0; + local $SIG{ALRM} = sub { + $timedOut = 1; + kill 'TERM', -$pid; + select(undef, undef, undef, 0.1); + kill 'KILL', -$pid; + }; + alarm($timeoutSecs); + + my $stdoutFd = fileno($stdoutRead); + my $stderrFd = fileno($stderrRead); + my $selector = IO::Select->new($stdoutRead, $stderrRead); + my %stream = ( + $stdoutFd => "", + $stderrFd => "", + ); + while ($selector->count()) { + for my $handle ($selector->can_read()) { + my $chunk = ""; + my $read = sysread($handle, $chunk, 65536); + if (not defined($read)) { + next if $! == EINTR; + die "unable to read tokenizer output: $!\n"; + } + if ($read == 0) { + $selector->remove($handle); + close($handle); + next; + } + $stream{fileno($handle)} .= $chunk; + } + } + + waitpid($pid, 0); + my $status = $?; + alarm(0); + + my $stdout = $stream{$stdoutFd}; + my $stderr = $stream{$stderrFd}; + if ($timedOut) { + $stderr .= "tokenize command timed out after [$timeoutSecs] seconds\n"; + return (124, "", $stderr); + } + + if ($status == -1) { + return (255, "", $stderr . "unable to wait for tokenizer: $!\n"); + } + my $signal = $status & 127; + if ($signal) { + return (128 + $signal, "", $stderr . "tokenizer killed by signal [$signal]\n"); + } + + return ($status >> 8, $stdout, $stderr); +} From e75edff53e36929ae9bf949e7a3105fc7c796534 Mon Sep 17 00:00:00 2001 From: Ellian Carlos Date: Fri, 25 Sep 2026 00:52:02 +0000 Subject: [PATCH 03/25] fix(tokenWorker): Silence perl exec warning on the failure fallback (cherry picked from commit f1c8362acbccb1a273be414ef33f9d432047dbb4) --- tokenizeByBlobId/tokenWorker.pl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tokenizeByBlobId/tokenWorker.pl b/tokenizeByBlobId/tokenWorker.pl index 7ed2fe7d..2864754f 100755 --- a/tokenizeByBlobId/tokenWorker.pl +++ b/tokenizeByBlobId/tokenWorker.pl @@ -162,7 +162,7 @@ sub run_command { print STDERR "unable to enter tokenizer directory [$workingDir]: $!\n"; _exit(255); }; - exec {$command[0]} @command; + { no warnings 'exec'; exec {$command[0]} @command; } print STDERR "unable to execute tokenizer [$command[0]]: $!\n"; _exit(255); } From 99eb30229af6756b0020b89b31491d99faa85e1f Mon Sep 17 00:00:00 2001 From: Ellian Carlos Date: Fri, 25 Sep 2026 03:21:16 +0000 Subject: [PATCH 04/25] ci: Run the tokenizer worker protocol tests (cherry picked from commit 6b6b97761f00d3007c2b550a4ec2c4db2171a6f6) --- .github/workflows/ci.yml | 3 +++ tests/test_token_worker.sh | 16 +++------------- 2 files changed, 6 insertions(+), 13 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 161affb5..9ad070eb 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -79,6 +79,9 @@ jobs: - name: Perl tests (prove) run: devenv shell -- bash -c 'prove tests/t' + - name: Tokenizer worker protocol tests + run: devenv shell -- bash tests/test_token_worker.sh + - name: Pipeline tests (shell) run: devenv shell -- bash -c 'tests/tokenize_gate.sh && tests/pipeline_workdir_guard.sh' diff --git a/tests/test_token_worker.sh b/tests/test_token_worker.sh index 79bfdbf7..26b1a034 100755 --- a/tests/test_token_worker.sh +++ b/tests/test_token_worker.sh @@ -1,22 +1,12 @@ #!/usr/bin/env bash - +# Byte-identity and lifecycle tests for tokenizeByBlobId/tokenWorker.pl. +# Run inside `devenv shell` (needs perl, python3, srcml, ctags on PATH). set -euo pipefail ROOT=$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd) -export PATH="/home/ellianco/Projects/cregit-workspace/cregit-issue61/.devenv/profile/bin:$PATH" SRCML2TOKEN="$ROOT/tokenize/srcMLtoken/srcml2token" -if [[ ! -x "$SRCML2TOKEN" ]]; then - if ! pkg-config --exists xerces-c; then - for candidate in /nix/store/*-xerces-c-*/lib/pkgconfig/xerces-c.pc; do - if [[ -f "$candidate" ]]; then - export PKG_CONFIG_PATH="$(dirname "$candidate")${PKG_CONFIG_PATH:+:$PKG_CONFIG_PATH}" - break - fi - done - fi - make -C "$ROOT/tokenize/srcMLtoken" -fi +[[ -x "$SRCML2TOKEN" ]] || make -C "$ROOT/tokenize/srcMLtoken" SRCML=$(command -v srcml) CTAGS=$(command -v ctags) From d60935ab04e88a9facfbf67cd26f45195500cc52 Mon Sep 17 00:00:00 2001 From: Ellian Carlos Date: Fri, 25 Sep 2026 03:58:15 +0000 Subject: [PATCH 05/25] perf(tokenWorker): Optional per-request trace via BFG_WORKER_TRACE One line per request (sha, bytes, ext, ms, exit, out bytes) appended to .. Off unless the variable is set. (cherry picked from commit 8fb7859609ec2a9358f3b9bc9c1befe7f2b97859) --- tokenizeByBlobId/tokenWorker.pl | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/tokenizeByBlobId/tokenWorker.pl b/tokenizeByBlobId/tokenWorker.pl index 2864754f..59b8a71b 100755 --- a/tokenizeByBlobId/tokenWorker.pl +++ b/tokenizeByBlobId/tokenWorker.pl @@ -11,6 +11,7 @@ use FindBin qw($RealBin); use IO::Select; use POSIX qw(_exit); +use Time::HiRes (); use lib "$RealBin/../tokenize"; use CregitLanguages; @@ -31,6 +32,12 @@ my $buildDir = "$RealBin/build"; make_path($buildDir) if not -d $buildDir; +my $trace; +if (my $tracePath = $ENV{BFG_WORKER_TRACE}) { + open($trace, '>>', "$tracePath.$$") or die "unable to open trace [$tracePath.$$]: $!\n"; + $trace->autoflush(1); +} + print STDOUT "READY\n"; while (defined(my $header = )) { @@ -44,6 +51,7 @@ my $contents = read_exact($bodyLen, "body"); my ($exitCode, $stdout, $stderr); + my $t0 = $trace ? Time::HiRes::time() : 0; my $ok = eval { ($exitCode, $stdout, $stderr) = process_request( $origSha, $filename, $fullPath, $contents, $timeoutSecs @@ -56,6 +64,11 @@ $stderr = $@ || "tokenizer request failed\n"; } + if ($trace) { + my $ext = $filename =~ /\.([^.]+)\z/ ? lc($1) : ""; + printf {$trace} "%s %d %s %.3f %d %d\n", + $origSha, $bodyLen, $ext, (Time::HiRes::time() - $t0) * 1000, $exitCode, length($stdout); + } print STDOUT "RES $exitCode " . length($stdout) . " " . length($stderr) . "\n"; print STDOUT $stdout, $stderr; } From 192395fc62e4e79f76f5eb963c748e5588dda83f Mon Sep 17 00:00:00 2001 From: Ellian Carlos Date: Fri, 25 Sep 2026 21:59:52 +0000 Subject: [PATCH 06/25] perf(tokenWorker): Run the srcML chain in-process The worker no longer spawns tokenize.pl -> bash -> tokenizeSrcMl.pl per blob. CregitSrcMl.pm reimplements tokenizeSrcMl.pl inside the worker: one persistent `ctags --_interactive --language-force=L` per language, one persistent `srcml2token --server`, and only srcml is exec'd per blob (writing its XML to a file the server parses). Per blob this removes a perl compile, a bash, a ctags exec and a srcml2token exec (which alone paid 3.45% of step-2 CPU to the dynamic linker). Equivalence evidence: - ctags: JSON interactive output converted to the `%n %N @@@ %K @@@ %S` xref records byte-for-byte on 632 C, 416 C++ and 600 Java real files plus edge cases (tabs/quotes/backslashes in signatures, UTF-8 and invalid bytes, anonymous structs, empty file). --sort=n is "unsorted" so emission order matches --sort=no. - srcml -o is byte-identical to srcml stdout. - worker output vs tokenBySha.pl reference: 1648/1648 real files identical (exit code and bytes); per-file 160-176 ms -> 26-55 ms. - Failure parity: srcml signal/non-zero exit, srcml2token failure and an empty token stream all return 33 with no output; timeout kills srcml and the persistent helpers and returns 124; the worker respawns helpers afterwards. srcml2token --server: `PARSE ` per line; answers tokens, then `\x01ERR ` lines and `\x01END `. Per-document state (the global line tables and the handler) is reset per request and the transcode leaks are closed, since the process now lives for the whole run. Existing `make test` expectations unchanged. CregitSrcMl.pm joins the srcML tokenizer identity. BFG_WORKER_INPROC=0 restores the spawn path for A/B measurement. (cherry picked from commit 830d91b7a8f984903cf161e8be59cb7805df67bd) --- tests/fixtures/Tiny.java | 4 + tests/fixtures/empty.c | 0 tests/fixtures/tiny.cpp | 2 + tests/test_token_worker.sh | 4 + tests/token_worker_driver.py | 45 +++ tokenize/CregitSrcMl.pm | 353 ++++++++++++++++++++ tokenize/srcMLtoken/srcml2token.cpp | 112 ++++--- tokenize/srcMLtoken/srcml2tokenHandlers.cpp | 31 +- tokenize/srcMLtoken/srcml2tokenHandlers.hpp | 4 + tokenizeByBlobId/tokenWorker.pl | 52 ++- 10 files changed, 550 insertions(+), 57 deletions(-) create mode 100644 tests/fixtures/Tiny.java create mode 100644 tests/fixtures/empty.c create mode 100644 tests/fixtures/tiny.cpp create mode 100644 tokenize/CregitSrcMl.pm diff --git a/tests/fixtures/Tiny.java b/tests/fixtures/Tiny.java new file mode 100644 index 00000000..c9b995f9 --- /dev/null +++ b/tests/fixtures/Tiny.java @@ -0,0 +1,4 @@ +package p; +public class Tiny { + int f(int a) { return a; } +} diff --git a/tests/fixtures/empty.c b/tests/fixtures/empty.c new file mode 100644 index 00000000..e69de29b diff --git a/tests/fixtures/tiny.cpp b/tests/fixtures/tiny.cpp new file mode 100644 index 00000000..88e4ae71 --- /dev/null +++ b/tests/fixtures/tiny.cpp @@ -0,0 +1,2 @@ +struct S { int m(int a) const { return a; } }; +template T id(T v) { return v; } diff --git a/tests/test_token_worker.sh b/tests/test_token_worker.sh index 26b1a034..ab88390f 100755 --- a/tests/test_token_worker.sh +++ b/tests/test_token_worker.sh @@ -20,4 +20,8 @@ python3 "$ROOT/tests/token_worker_driver.py" \ --tokenize-command "$TOKENIZE_COMMAND" \ --fixture "$ROOT/tests/fixtures/tiny.c" \ --fixture "$ROOT/tests/fixtures/quote'name.c" \ + --fixture "$ROOT/tests/fixtures/tiny.cpp" \ + --fixture "$ROOT/tests/fixtures/Tiny.java" \ + --fixture "$ROOT/tests/fixtures/empty.c" \ + --in-process \ --temp-root "$TEMP_ROOT" diff --git a/tests/token_worker_driver.py b/tests/token_worker_driver.py index 1e4b8cb5..7d95fe4e 100755 --- a/tests/token_worker_driver.py +++ b/tests/token_worker_driver.py @@ -105,6 +105,47 @@ def assert_no_timeout_process(marker): ) +def in_process_checks(args, marker, expected_first): + real_srcml = subprocess.run(["which", "srcml"], stdout=subprocess.PIPE, check=True).stdout.decode().strip() + fake_srcml = args.temp_root / "fake-srcml.sh" + fake_srcml.write_text( + "#!/usr/bin/env bash\n" + 'src="${@: -3:1}"\n' + f'grep -q SLEEP_MARKER "$src" && exec -a {marker} sleep 100\n' + 'grep -q ABORT_MARKER "$src" && kill -ABRT $$\n' + f'exec {real_srcml} "$@"\n', + encoding="utf-8", + ) + fake_srcml.chmod(0o755) + memo = args.temp_root / "inproc-memo" + memo.mkdir() + command = f"{args.tokenize_command} --srcml={fake_srcml}".replace( + f"--srcml={real_srcml} ", "") + worker = Worker(args.worker, memo, command) + + started = time.monotonic() + timed_out = worker.request("timeout.c", "fixtures/timeout.c", b"int SLEEP_MARKER;\n", 1) + elapsed = time.monotonic() - started + if timed_out[0] != 124 or elapsed > 3.0: + raise AssertionError(f"in-process timeout response was {timed_out!r} after {elapsed:.2f}s") + assert_no_timeout_process(marker) + print(f"PASS in-process timeout exits 124 in {elapsed:.2f}s with no child left") + + crash = worker.request("crash.c", "fixtures/crash.c", b"int ABORT_MARKER;\n", 20) + if crash[0] != 33 or b"killed by signal 6" not in crash[2]: + raise AssertionError(f"srcml death was not reported as a parser crash: {crash!r}") + print("PASS in-process srcml crash exits 33 without output") + + if any(memo.rglob("*")): + raise AssertionError("failed in-process requests were memoized") + recovered = worker.request(args.fixture[0].name, f"fixtures/{args.fixture[0].name}", + args.fixture[0].read_bytes(), 20) + if recovered != (0, expected_first, b""): + raise AssertionError(f"in-process worker did not recover: {recovered!r}") + worker.close() + print("PASS in-process worker recovers after timeout and crash") + + def main(): parser = argparse.ArgumentParser() parser.add_argument("--worker", required=True, type=pathlib.Path) @@ -112,6 +153,7 @@ def main(): parser.add_argument("--tokenize-command", required=True) parser.add_argument("--fixture", action="append", required=True, type=pathlib.Path) parser.add_argument("--temp-root", required=True, type=pathlib.Path) + parser.add_argument("--in-process", action="store_true") args = parser.parse_args() worker_memo = args.temp_root / "worker-memo" @@ -189,6 +231,9 @@ def main(): timeout_worker.close() print(f"PASS timeout exits 124 in {elapsed:.2f}s with no child left") + if args.in_process: + in_process_checks(args, marker, expected_outputs[0]) + print("ALL TOKEN WORKER TESTS PASSED") diff --git a/tokenize/CregitSrcMl.pm b/tokenize/CregitSrcMl.pm new file mode 100644 index 00000000..7bae085b --- /dev/null +++ b/tokenize/CregitSrcMl.pm @@ -0,0 +1,353 @@ +package CregitSrcMl; + +# In-process replacement for tokenizeSrcMl.pl: one persistent ctags per language +# and one persistent `srcml2token --server`; only srcml is spawned per file. + +use strict; +use warnings; +use bytes; +use Errno qw(EINTR); +use POSIX qw(_exit); + +our $PARSER_CRASH_EXIT = 33; + +sub new { + my ($class, %options) = @_; + my $self = { + srcml => $options{srcml} // "srcml", + srcml2token => $options{srcml2token} // "srcml2token", + ctags => $options{ctags} // "ctags-universal", + position => $options{position} // 0, + ctagsProc => {}, + s2t => undef, + child => undef, + timedOut => 0, + }; + return bless $self, $class; +} + +sub tokenize { + my ($self, $language, $inputName, $workDir, $timeoutSecs) = @_; + + $self->{timedOut} = 0; + local $SIG{ALRM} = sub { + $self->{timedOut} = 1; + $self->kill_all(); + }; + alarm($timeoutSecs) if $timeoutSecs; + + my @result; + my $ok = eval { + @result = $self->run($language, $inputName, $workDir); + 1; + }; + my $error = $@; + alarm(0); + + if ($self->{timedOut}) { + $self->shutdown(); + return (124, "", "tokenize command timed out after [$timeoutSecs] seconds\n"); + } + return (255, "", $error || "srcml tokenization failed\n") if not $ok; + return @result; +} + +sub run { + my ($self, $language, $inputName, $workDir) = @_; + my $stderr = ""; + + my ($declarations, $listDeclarations) = + $self->read_declarations($language, $inputName, "$workDir/$inputName"); + + my $xmlPath = "$workDir/srcml-output.xml"; + my ($srcmlStatus, $srcmlErr) = $self->run_srcml($language, $inputName, $workDir, $xmlPath); + $stderr .= $srcmlErr; + if ($srcmlStatus & 127) { + my $signal = $srcmlStatus & 127; + return parser_crash($inputName, $stderr, + "srcml was killed by signal $signal (shell status " . (128 + $signal) . "). " + . "srcML 1.1.0 dies on a signal on some C/C++ inputs when --position " + . "is given; --position cannot be dropped because the token format " + . "depends on it."); + } + if ($srcmlStatus != 0) { + return parser_crash($inputName, $stderr, "srcml exited " . ($srcmlStatus >> 8) . "."); + } + + my ($tokenStatus, $tokenLines, $tokenErr) = $self->parse_xml($xmlPath); + unlink $xmlPath; + $stderr .= $tokenErr; + if (not defined $tokenStatus) { + return parser_crash($inputName, $stderr, "srcml2token died before answering."); + } + if ($tokenStatus != 0) { + return parser_crash($inputName, $stderr, "srcml2token exited $tokenStatus."); + } + if (not @$tokenLines) { + return parser_crash($inputName, $stderr, + "the token stream was empty. A successful srcML parse always emits at " + . "least the begin_unit/end_unit wrapper, even for an empty file, so an " + . "empty stream is a failed parse and never a legitimately empty result."); + } + + my $output = $self->merge($tokenLines, $declarations, $listDeclarations); + return (0, $output, $stderr); +} + +sub merge { + my ($self, $tokenLines, $declarations, $listDeclarations) = @_; + my $position = $self->{position}; + my $lastLine = -1; + my $output = ""; + + for my $tokenLine (@$tokenLines) { + die "unable to parse srcml line [$tokenLine]" + unless $tokenLine =~ /^([0-9]+|-):([0-9]+|-)\s+(.+)$/; + my ($line, $col, $token) = ($1, $2, $3); + my $lineNum = $line eq '-' ? 0 : $line; + if ($lineNum != $lastLine) { + my $names = $listDeclarations->{$line}; + for my $name (@{ $names || [] }) { + my $decl = $declarations->{$line}{$name}; + die "Illegal value in get declaration [$line][$name]" unless defined $decl; + $output .= "$line:-|" if $position; + $output .= "DECL|$decl->{type}|$decl->{name}\n"; + } + } + $lastLine = $lineNum; + $output .= "$line:$col|" if $position; + $output .= "$token\n"; + if ($token =~ /^end_/) { + $output .= "-:-|" if $position; + $output .= "\n"; + } + } + return $output; +} + +sub parser_crash { + my ($filename, $stderr, $why) = @_; + $stderr .= "cregit: tokenization of [$filename] FAILED: $why\n"; + $stderr .= "cregit: refusing to emit a tokenization for [$filename]; " + . "exiting $PARSER_CRASH_EXIT so the caller can count this blob.\n"; + return ($PARSER_CRASH_EXIT, "", $stderr); +} + +# --- ctags ------------------------------------------------------------------- + +sub ctags_for { + my ($self, $language) = @_; + my $proc = $self->{ctagsProc}{$language}; + return $proc if $proc; + + $proc = spawn_pipe([ $self->{ctags}, "--_interactive", "--language-force=$language", + "--fields=+nKS", "--sort=no" ]); + my $hello = readline($proc->{out}); + die "ctags did not start: " . ($hello // "EOF") . "\n" + unless defined $hello and $hello =~ /"_type": "program"/; + $self->{ctagsProc}{$language} = $proc; + return $proc; +} + +sub read_declarations { + my ($self, $language, $inputName, $path) = @_; + my (%declarations, %listDeclarations); + + my $source = do { + open(my $fh, '<:raw', $path) or die "unable to read [$path]: $!\n"; + local $/; + <$fh>; + }; + $source = "" unless defined $source; + + my $proc = $self->ctags_for($language); + my $in = $proc->{in}; + print {$in} qq({"command":"generate-tags","filename":"$inputName","size":) + . length($source) . "}\n", $source; + $in->flush; + + while (1) { + my $line = readline($proc->{out}); + if (not defined $line) { + $self->drop_ctags($language); + last; + } + last if $line =~ /"_type": "completed"/; + if ($line =~ /"_type": "error"/) { + $self->drop_ctags($language); + die "ctags rejected the request: $line"; + } + next unless $line =~ /"_type": "tag"/; + + my ($n) = $line =~ /"line": ([0-9]+)/; + my $name = json_field($line, "name"); + my $kind = json_field($line, "kind"); + my $sig = json_field($line, "signature"); + next unless defined $n and defined $name and defined $kind; + $sig = "-" unless defined $sig; + for ($name, $kind, $sig) { s/\\/\\\\/g } + + my $xref = "$n $name @@@ $kind @@@ $sig"; + die "unable to parse output [$xref]" unless $xref =~ /^([0-9]+) (.+) @@@ (.*) @@@ (.*)$/; + my %decl = (line => $1, name => $2, type => $3, sig => $4); + $decl{name} .= " " . $decl{sig} if $decl{sig} ne "-"; + $declarations{$decl{line}}{$decl{name}} = \%decl; + push @{ $listDeclarations{$decl{line}} }, $decl{name}; + } + return (\%declarations, \%listDeclarations); +} + +sub drop_ctags { + my ($self, $language) = @_; + my $proc = delete $self->{ctagsProc}{$language}; + reap_pipe($proc) if $proc; +} + +sub json_str { + my ($s) = @_; + $s =~ s/\\(["\\\/bfnrt]|u[0-9a-fA-F]{4})/ + my $e = $1; + $e eq '"' ? '"' : $e eq '\\' ? '\\' : $e eq '\/' ? '\/' : + $e eq 'b' ? "\b" : $e eq 'f' ? "\f" : $e eq 'n' ? "\n" : + $e eq 'r' ? "\r" : $e eq 't' ? "\t" : chr(hex(substr($e, 1)))/ge; + return $s; +} + +sub json_field { + my ($line, $key) = @_; + return undef unless $line =~ /"\Q$key\E": "((?:[^"\\]|\\.)*)"/; + return json_str($1); +} + +# --- srcml ------------------------------------------------------------------- + +sub run_srcml { + my ($self, $language, $inputName, $workDir, $xmlPath) = @_; + pipe(my $errRead, my $errWrite) or die "unable to create stderr pipe: $!\n"; + + my $pid = fork(); + die "unable to fork srcml: $!\n" if not defined $pid; + if ($pid == 0) { + close($errRead); + open(STDIN, '<', '/dev/null'); + open(STDOUT, '>', '/dev/null'); + open(STDERR, '>&', $errWrite) or _exit(255); + close($errWrite); + setpgrp(0, 0); + chdir($workDir) or do { print STDERR "unable to enter [$workDir]: $!\n"; _exit(255) }; + { no warnings 'exec'; exec $self->{srcml}, "-l", $language, "--position", $inputName, "-o", $xmlPath; } + print STDERR "unable to execute srcml [$self->{srcml}]: $!\n"; + _exit(255); + } + close($errWrite); + $self->{child} = $pid; + + my $stderr = ""; + while (1) { + my $read = sysread($errRead, $stderr, 65536, length($stderr)); + if (not defined $read) { + next if $! == EINTR; + last; + } + last if $read == 0; + } + close($errRead); + waitpid($pid, 0); + my $status = $?; + $self->{child} = undef; + return ($status, $stderr); +} + +# --- srcml2token ------------------------------------------------------------- + +sub s2t { + my ($self) = @_; + return $self->{s2t} if $self->{s2t}; + $self->{s2t} = spawn_pipe([ $self->{srcml2token}, "--server" ]); + return $self->{s2t}; +} + +sub parse_xml { + my ($self, $xmlPath) = @_; + my $proc = $self->s2t(); + my $in = $proc->{in}; + print {$in} "PARSE $xmlPath\n"; + $in->flush; + + my (@tokens, $status); + my $stderr = ""; + while (1) { + my $line = readline($proc->{out}); + if (not defined $line) { + reap_pipe(delete $self->{s2t}); + return (undef, \@tokens, $stderr); + } + if ($line =~ /\A\x01END ([0-9]+)\n\z/) { + $status = $1; + last; + } + if ($line =~ /\A\x01ERR (.*)\n\z/s) { + $stderr .= "$1\n"; + next; + } + chomp $line; + push @tokens, $line; + } + return ($status, \@tokens, $stderr); +} + +# --- process plumbing -------------------------------------------------------- + +sub spawn_pipe { + my ($command) = @_; + pipe(my $childIn, my $parentOut) or die "unable to create pipe: $!\n"; + pipe(my $parentIn, my $childOut) or die "unable to create pipe: $!\n"; + + my $pid = fork(); + die "unable to fork [$command->[0]]: $!\n" if not defined $pid; + if ($pid == 0) { + close($parentOut); + close($parentIn); + open(STDIN, '<&', $childIn) or _exit(255); + open(STDOUT, '>&', $childOut) or _exit(255); + close($childIn); + close($childOut); + { no warnings 'exec'; exec @$command; } + print STDERR "unable to execute [$command->[0]]: $!\n"; + _exit(255); + } + close($childIn); + close($childOut); + binmode $parentOut; + binmode $parentIn; + return { pid => $pid, in => $parentOut, out => $parentIn }; +} + +sub reap_pipe { + my ($proc) = @_; + return unless $proc; + close($proc->{in}) if defined fileno($proc->{in}); + close($proc->{out}) if defined fileno($proc->{out}); + kill 'TERM', $proc->{pid}; + waitpid($proc->{pid}, 0); +} + +sub kill_all { + my ($self) = @_; + if (my $pid = $self->{child}) { + kill 'TERM', -$pid; + select(undef, undef, undef, 0.1); + kill 'KILL', -$pid; + } + for my $proc (values %{ $self->{ctagsProc} }, $self->{s2t}) { + kill 'KILL', $proc->{pid} if $proc; + } +} + +sub shutdown { + my ($self) = @_; + reap_pipe(delete $self->{ctagsProc}{$_}) for keys %{ $self->{ctagsProc} }; + reap_pipe(delete $self->{s2t}); +} + +1; diff --git a/tokenize/srcMLtoken/srcml2token.cpp b/tokenize/srcMLtoken/srcml2token.cpp index a91f110e..60702dc6 100644 --- a/tokenize/srcMLtoken/srcml2token.cpp +++ b/tokenize/srcMLtoken/srcml2token.cpp @@ -22,6 +22,8 @@ #include #include "srcml2token.hpp" #include +#include +#include // --------------------------------------------------------------------------- // Local helper methods @@ -29,11 +31,71 @@ void usage() { XERCES_STD_QUALIFIER cout << "\nUsage:\n" - " srcml2token \n\n" + " srcml2token \n" + " srcml2token --server\n\n" "This program converts the output of srcML into a simplified tokenized version\n" + "--server reads `PARSE ` lines on stdin and answers each with the tokens,\n" + "then `\\x01ERR ` lines and a final `\\x01END ` line.\n" << XERCES_STD_QUALIFIER endl; } +static int parseOne(SAX2XMLReader* parser, const char* path) +{ + srcml2tokenResetState(); + srcml2tokenHandlers handler; + parser->setContentHandler(&handler); + parser->setErrorHandler(&handler); + try + { + if (path == NULL) { + StdInInputSource src; + parser->parse(src); + } else { + parser->parse(path); + } + } + catch (const OutOfMemoryException&) + { + *srcml2tokenDiag << "OutOfMemoryException" << XERCES_STD_QUALIFIER endl; + return 4; + } + catch (const XMLException& e) + { + *srcml2tokenDiag << "\nError during parsing: \n" + << StrX(e.getMessage()) + << "\n" << XERCES_STD_QUALIFIER endl; + return 4; + } + catch (...) + { + *srcml2tokenDiag << "Unexpected exception during parsing" << XERCES_STD_QUALIFIER endl; + return 4; + } + return 0; +} + +static int serverLoop(SAX2XMLReader* parser) +{ + std::string line; + while (std::getline(std::cin, line)) { + if (line.compare(0, 6, "PARSE ") != 0) { + std::cout << "\x01" "ERR malformed request [" << line << "]\n" "\x01" "END 2\n" << std::flush; + continue; + } + std::ostringstream diag; + srcml2tokenDiag = &diag; + int status = parseOne(parser, line.c_str() + 6); + srcml2tokenDiag = &std::cerr; + + std::istringstream lines(diag.str()); + std::string d; + while (std::getline(lines, d)) { + std::cout << "\x01" "ERR " << d << "\n"; + } + std::cout << "\x01" "END " << status << "\n" << std::flush; + } + return 0; +} // --------------------------------------------------------------------------- // Program entry point @@ -56,42 +118,11 @@ int main(int argC, char* argV[]) SAX2XMLReader* parser = XMLReaderFactory::createXMLReader(); - srcml2tokenHandlers handler; - parser->setContentHandler(&handler); - parser->setErrorHandler(&handler); - - int errorCount = 0; - // create a faux scope so that 'src' destructor is called before - // XMLPlatformUtils::Terminate - { - // - // Kick off the parse and catch any exceptions. Create a standard - // input input source and tell the parser to parse from that. - // - try - { - if (argC < 2) { - StdInInputSource src; - parser->parse(src); - } else { - parser->parse(argV[1]); - } - } - catch (const OutOfMemoryException&) - { - XERCES_STD_QUALIFIER cerr << "OutOfMemoryException" << XERCES_STD_QUALIFIER endl; - errorCount = 2; - return 4; - } - catch (const XMLException& e) - { - XERCES_STD_QUALIFIER cerr << "\nError during parsing: \n" - << StrX(e.getMessage()) - << "\n" << XERCES_STD_QUALIFIER endl; - errorCount = 1; - return 4; - } - + int status; + if (argC >= 2 && strcmp(argV[1], "--server") == 0) { + status = serverLoop(parser); + } else { + status = parseOne(parser, argC < 2 ? NULL : argV[1]); } // @@ -101,10 +132,5 @@ int main(int argC, char* argV[]) XMLPlatformUtils::Terminate(); - if (errorCount > 0) - return 4; - else - return 0; + return status; } - - diff --git a/tokenize/srcMLtoken/srcml2tokenHandlers.cpp b/tokenize/srcMLtoken/srcml2tokenHandlers.cpp index a6ba5884..153e1713 100644 --- a/tokenize/srcMLtoken/srcml2tokenHandlers.cpp +++ b/tokenize/srcMLtoken/srcml2tokenHandlers.cpp @@ -36,14 +36,25 @@ std::vector lineMarkers {0}; std::vector lineNumbers {1}; +std::ostream* srcml2tokenDiag = &std::cerr; + +void srcml2tokenResetState() { + lineMarkers.assign(1, 0); + lineNumbers.assign(1, 1); +} + +static std::string transcoded(const XMLCh* s) { + char* local = XMLString::transcode(s); + std::string result = local == NULL ? "" : local; + XMLString::release(&local); + return result; +} std::string get_attribute_value(const Attributes& attrs, std::string name) { XMLCh* revName= XMLString::transcode(name.c_str()); - char* st = XMLString::transcode(attrs.getValue(revName)); - std::string result = ""; - if (st != NULL) - result = st; + std::string result = transcoded(attrs.getValue(revName)); + XMLString::release(&revName); return result; } @@ -152,7 +163,7 @@ void srcml2tokenHandlers::startElement(const XMLCh* const //uri , const XMLCh* const qname , const Attributes& attrs) { - std::string tagLocal = XMLString::transcode(localname); + std::string tagLocal = transcoded(localname); // char *tagName = XMLString::transcode(qname); // std::string savePos = position(attrs); std::string savePos = newGetPosition(); @@ -205,7 +216,7 @@ void srcml2tokenHandlers::endElement (const XMLCh *const /*uri*/, const XMLCh *const localname, const XMLCh *const /*qname*/) { - char *tagName = XMLString::transcode(localname); + std::string tagName = transcoded(localname); // No escapes are legal here if (currentContent.length() > 0 ) { @@ -230,7 +241,7 @@ void srcml2tokenHandlers::endElement (const XMLCh *const /*uri*/, void srcml2tokenHandlers::characters( const XMLCh* const chars , const XMLSize_t length) { - std::string original = XMLString::transcode(chars); + std::string original = transcoded(chars); std::string st = mytrim(original); std::string node = mystack.top(); @@ -265,7 +276,7 @@ void srcml2tokenHandlers::startDocument() // --------------------------------------------------------------------------- void srcml2tokenHandlers::error(const SAXParseException& e) { - XERCES_STD_QUALIFIER cerr << "\nError at file " << StrX(e.getSystemId()) + *srcml2tokenDiag << "\nError at file " << StrX(e.getSystemId()) << ", line " << e.getLineNumber() << ", char " << e.getColumnNumber() << "\n Message: " << StrX(e.getMessage()) << XERCES_STD_QUALIFIER endl; @@ -273,7 +284,7 @@ void srcml2tokenHandlers::error(const SAXParseException& e) void srcml2tokenHandlers::fatalError(const SAXParseException& e) { - XERCES_STD_QUALIFIER cerr << "\nFatal Error at file " << StrX(e.getSystemId()) + *srcml2tokenDiag << "\nFatal Error at file " << StrX(e.getSystemId()) << ", line " << e.getLineNumber() << ", char " << e.getColumnNumber() << "\n Message: " << StrX(e.getMessage()) << XERCES_STD_QUALIFIER endl; @@ -281,7 +292,7 @@ void srcml2tokenHandlers::fatalError(const SAXParseException& e) void srcml2tokenHandlers::warning(const SAXParseException& e) { - XERCES_STD_QUALIFIER cerr << "\nWarning at file " << StrX(e.getSystemId()) + *srcml2tokenDiag << "\nWarning at file " << StrX(e.getSystemId()) << ", line " << e.getLineNumber() << ", char " << e.getColumnNumber() << "\n Message: " << StrX(e.getMessage()) << XERCES_STD_QUALIFIER endl; diff --git a/tokenize/srcMLtoken/srcml2tokenHandlers.hpp b/tokenize/srcMLtoken/srcml2tokenHandlers.hpp index 256f5178..70649942 100644 --- a/tokenize/srcMLtoken/srcml2tokenHandlers.hpp +++ b/tokenize/srcMLtoken/srcml2tokenHandlers.hpp @@ -20,6 +20,7 @@ // --------------------------------------------------------------------------- // Includes // --------------------------------------------------------------------------- +#include #include #include #include @@ -27,6 +28,9 @@ XERCES_CPP_NAMESPACE_USE +extern std::ostream* srcml2tokenDiag; +void srcml2tokenResetState(); + class srcml2tokenHandlers : public DefaultHandler { public: diff --git a/tokenizeByBlobId/tokenWorker.pl b/tokenizeByBlobId/tokenWorker.pl index 59b8a71b..a632577d 100755 --- a/tokenizeByBlobId/tokenWorker.pl +++ b/tokenizeByBlobId/tokenWorker.pl @@ -12,12 +12,15 @@ use IO::Select; use POSIX qw(_exit); use Time::HiRes (); +use File::Basename qw(basename dirname); use lib "$RealBin/../tokenize"; use CregitLanguages; +use CregitSrcMl; binmode STDIN; binmode STDOUT; $| = 1; +$SIG{PIPE} = 'IGNORE'; my $memoDir = $ENV{BFG_MEMO_DIR}; die "You must define BFG_MEMO_DIR\n" if not defined($memoDir) or $memoDir eq ""; @@ -32,6 +35,8 @@ my $buildDir = "$RealBin/build"; make_path($buildDir) if not -d $buildDir; +my $inProcess = in_process_tokenizer(@tokenizeCommand); + my $trace; if (my $tracePath = $ENV{BFG_WORKER_TRACE}) { open($trace, '>>', "$tracePath.$$") or die "unable to open trace [$tracePath.$$]: $!\n"; @@ -73,8 +78,39 @@ print STDOUT $stdout, $stderr; } +$inProcess->shutdown() if $inProcess; exit 0; +# In-process srcML only for the stock dispatcher/parser with options the worker honors. +sub in_process_tokenizer { + my ($program, @options) = @_; + return undef if ($ENV{BFG_WORKER_INPROC} // "1") eq "0"; + return undef unless defined $program; + my $name = basename($program); + return undef unless $name eq "tokenize.pl" or $name eq $CregitLanguages::SRCML_PARSER_REL; + + my %config = ( + srcml => "srcml", + srcml2token => dirname($program) . "/srcMLtoken/srcml2token", + ctags => "ctags-universal", + position => 0, + ); + for my $option (@options) { + if ($option =~ /\A--srcml=(.+)\z/) { $config{srcml} = $1 } + elsif ($option =~ /\A--srcml2token=(.+)\z/) { $config{srcml2token} = $1 } + elsif ($option =~ /\A--ctags=(.+)\z/) { $config{ctags} = $1 } + elsif ($option eq "--position") { $config{position} = 1 } + else { return undef } + } + return CregitSrcMl->new(%config); +} + +sub srcml_language { + my ($language) = @_; + my $parser = $CregitLanguages::LANG_PARSER_REL{$language}; + return (defined $parser and $parser eq $CregitLanguages::SRCML_PARSER_REL); +} + sub read_exact { my ($length, $label) = @_; my $value = ""; @@ -129,10 +165,18 @@ sub process_request { print {$input} $contents; close($input) or die "unable to close temp input [$inputFile]: $!\n"; - my @command = (@tokenizeCommand, "--language=$mapLang{$fileExt}", $inputName); - my ($exitCode, $output, $error) = run_command( - "$tempDir", $timeoutSecs, @command - ); + my $language = $mapLang{$fileExt}; + my ($exitCode, $output, $error); + if ($inProcess and srcml_language($language)) { + ($exitCode, $output, $error) = $inProcess->tokenize( + $language, $inputName, "$tempDir", $timeoutSecs + ); + } else { + my @command = (@tokenizeCommand, "--language=$language", $inputName); + ($exitCode, $output, $error) = run_command( + "$tempDir", $timeoutSecs, @command + ); + } return ($exitCode, "", $error) if $exitCode != 0; make_path($dir) if not -d $dir; From 5e6623c729ea52f556ff3fa28059d884c6518e4a Mon Sep 17 00:00:00 2001 From: Ellian Carlos Date: Mon, 28 Sep 2026 13:31:07 +0000 Subject: [PATCH 07/25] fix: restore the exec bit on tokenize.pl and tokenBySha.pl 7db5f8e dropped it from both files. blobExec runs tokenBySha.pl as a command, and tokenBySha.pl runs tokenize.pl the same way, so both must be executable. The tokenizer worker test also runs them directly. --- tokenize/tokenize.pl | 0 tokenizeByBlobId/tokenBySha.pl | 0 2 files changed, 0 insertions(+), 0 deletions(-) mode change 100644 => 100755 tokenize/tokenize.pl mode change 100644 => 100755 tokenizeByBlobId/tokenBySha.pl diff --git a/tokenize/tokenize.pl b/tokenize/tokenize.pl old mode 100644 new mode 100755 diff --git a/tokenizeByBlobId/tokenBySha.pl b/tokenizeByBlobId/tokenBySha.pl old mode 100644 new mode 100755 From 4d8cd870912b3e12ea3efcb6e22fb647ebde8732 Mon Sep 17 00:00:00 2001 From: Ellian Carlos Date: Mon, 28 Sep 2026 13:31:58 +0000 Subject: [PATCH 08/25] fix(tokenWorker): route .C and .H to C++, as tokenBySha.pl does The worker lowercased the extension before the lookup, so upper.H was tokenized as C. master's tokenBySha.pl asks CregitLanguages::language_for_ext, which keeps .C and .H as C++. The worker now asks the same function. The new upper.H fixture checks it. --- tests/fixtures/upper.H | 6 ++++++ tests/test_token_worker.sh | 1 + tokenizeByBlobId/tokenWorker.pl | 11 ++++++----- 3 files changed, 13 insertions(+), 5 deletions(-) create mode 100644 tests/fixtures/upper.H diff --git a/tests/fixtures/upper.H b/tests/fixtures/upper.H new file mode 100644 index 00000000..e039d96f --- /dev/null +++ b/tests/fixtures/upper.H @@ -0,0 +1,6 @@ +namespace n { +class A { +public: + int x; +}; +} diff --git a/tests/test_token_worker.sh b/tests/test_token_worker.sh index ab88390f..37868ecf 100755 --- a/tests/test_token_worker.sh +++ b/tests/test_token_worker.sh @@ -23,5 +23,6 @@ python3 "$ROOT/tests/token_worker_driver.py" \ --fixture "$ROOT/tests/fixtures/tiny.cpp" \ --fixture "$ROOT/tests/fixtures/Tiny.java" \ --fixture "$ROOT/tests/fixtures/empty.c" \ + --fixture "$ROOT/tests/fixtures/upper.H" \ --in-process \ --temp-root "$TEMP_ROOT" diff --git a/tokenizeByBlobId/tokenWorker.pl b/tokenizeByBlobId/tokenWorker.pl index a632577d..a5e37253 100755 --- a/tokenizeByBlobId/tokenWorker.pl +++ b/tokenizeByBlobId/tokenWorker.pl @@ -31,7 +31,6 @@ if not defined($tokenizeCmd) or $tokenizeCmd eq ""; my @tokenizeCommand = grep { length($_) } split /\s+/, $tokenizeCmd; -my %mapLang = %CregitLanguages::EXT_LANG; my $buildDir = "$RealBin/build"; make_path($buildDir) if not -d $buildDir; @@ -133,9 +132,12 @@ sub protocol_error { sub process_request { my ($origSha, $filename, $fullPath, $contents, $timeoutSecs) = @_; - my $fileExt; - $fileExt = lc($1) if $filename =~ /\.([^.]+)\z/; - if (not defined($fileExt) or not defined($mapLang{$fileExt})) { + my ($fileExt, $language); + if ($filename =~ /\.([^.]+)\z/) { + $fileExt = lc($1); + $language = CregitLanguages::language_for_ext($1); + } + if (not defined($language)) { my $shownExt = defined($fileExt) ? $fileExt : ""; return (255, "", "unknown file extension [$shownExt]\n"); } @@ -165,7 +167,6 @@ sub process_request { print {$input} $contents; close($input) or die "unable to close temp input [$inputFile]: $!\n"; - my $language = $mapLang{$fileExt}; my ($exitCode, $output, $error); if ($inProcess and srcml_language($language)) { ($exitCode, $output, $error) = $inProcess->tokenize( From c439ea47c6b9b84e3a0522522241a5bd9b4030c3 Mon Sep 17 00:00:00 2001 From: Ellian Carlos Date: Mon, 28 Sep 2026 18:15:16 +0000 Subject: [PATCH 09/25] fix(tests): name the timeout process without exec -a The worker tests started `sleep` with `exec -a `, so pgrep could find it. The Nix coreutils `sleep` is a link to one coreutils binary, and that binary picks the program from argv[0]. With the marker as argv[0] it printed "unknown program" and exited 1, so the timeout test failed in CI. /usr/bin/sleep on the local host is a separate binary, so the test passed there. Run `perl -e "sleep 100" ` instead. The marker is now an argument, so pgrep -f still finds it, and argv[0] stays correct. --- tests/token_worker_driver.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/token_worker_driver.py b/tests/token_worker_driver.py index 7d95fe4e..abdb8203 100755 --- a/tests/token_worker_driver.py +++ b/tests/token_worker_driver.py @@ -111,7 +111,7 @@ def in_process_checks(args, marker, expected_first): fake_srcml.write_text( "#!/usr/bin/env bash\n" 'src="${@: -3:1}"\n' - f'grep -q SLEEP_MARKER "$src" && exec -a {marker} sleep 100\n' + f'grep -q SLEEP_MARKER "$src" && exec perl -e "sleep 100" {marker}\n' 'grep -q ABORT_MARKER "$src" && kill -ABRT $$\n' f'exec {real_srcml} "$@"\n', encoding="utf-8", @@ -213,7 +213,7 @@ def main(): marker = f"cregit-token-worker-timeout-{os.getpid()}" timeout_script.write_text( "#!/usr/bin/env bash\n" - f"exec -a {marker} sleep 100\n", + f"exec perl -e 'sleep 100' {marker}\n", encoding="utf-8", ) timeout_script.chmod(0o755) From 5db0fc1decf40eab96780aab5bf3449fba2fd89b Mon Sep 17 00:00:00 2001 From: Ellian Carlos Date: Sun, 4 Oct 2026 14:10:45 +0000 Subject: [PATCH 10/25] fix(blobExec): clear worker diagnostics per request The pool kept every stderr line of a worker for the whole run and read only the part after an offset. Each worker lives for the whole run, and its ctags and srcml2token helpers write to the same stderr, so the buffer had no upper limit on a large corpus. Clear the buffer when a request starts. A failed request still reports the lines of that request, and the offset bookkeeping goes away. --- .../cregit/blobexec/TokenizerWorkerPool.scala | 20 +++++++------------ 1 file changed, 7 insertions(+), 13 deletions(-) diff --git a/blobExec/src/main/scala/cregit/blobexec/TokenizerWorkerPool.scala b/blobExec/src/main/scala/cregit/blobexec/TokenizerWorkerPool.scala index e0171aee..6786b63e 100644 --- a/blobExec/src/main/scala/cregit/blobexec/TokenizerWorkerPool.scala +++ b/blobExec/src/main/scala/cregit/blobexec/TokenizerWorkerPool.scala @@ -33,10 +33,8 @@ final class TokenizerWorkerPool( diagnostics.append(line).append('\n') } - def diagnosticsLength: Int = diagnostics.synchronized(diagnostics.length) - - def diagnosticsSince(offset: Int): String = diagnostics.synchronized { - diagnostics.substring(math.min(offset, diagnostics.length)) + def takeDiagnostics(): String = diagnostics.synchronized { + try diagnostics.toString finally diagnostics.clear() } } @@ -65,12 +63,12 @@ final class TokenizerWorkerPool( if (closed.get()) throw new IllegalStateException("tokenizer worker pool is closed") val worker = idle.take() - val diagnosticOffset = worker.diagnosticsLength + worker.takeDiagnostics() try { writeRequest(worker, bytes, origSha, filename, fullPath) } catch { case failure: IOException => - return failedWorker(worker, diagnosticOffset, failure) + return failedWorker(worker, failure) } val finished = new CountDownLatch(1) @@ -98,7 +96,7 @@ final class TokenizerWorkerPool( ChildRunner.Outcome.Killed(s"no exit within ${timeoutSeconds}s") else ChildRunner.Outcome.Exited(result.exitCode, result.stdout, result.stderr) case Left(failure) => - failedWorker(worker, diagnosticOffset, failure) + failedWorker(worker, failure) } } } @@ -248,13 +246,9 @@ final class TokenizerWorkerPool( length } - private def failedWorker( - worker: Worker, - diagnosticOffset: Int, - failure: Throwable - ): ChildRunner.Outcome = { + private def failedWorker(worker: Worker, failure: Throwable): ChildRunner.Outcome = { val workerExit = if (worker.process.waitFor(100, TimeUnit.MILLISECONDS)) Some(worker.process.exitValue()) else None - val diagnostics = worker.diagnosticsSince(diagnosticOffset) + val diagnostics = worker.takeDiagnostics() replaceWorker(worker, force = true) if (workerExit.contains(WorkerTimeoutExitCode)) ChildRunner.Outcome.Killed(s"worker exited $WorkerTimeoutExitCode (timeout)") From 360f4d95a421cd83e36e1faf59dace32b7e6f89c Mon Sep 17 00:00:00 2001 From: Ellian Carlos Date: Sun, 4 Oct 2026 14:12:25 +0000 Subject: [PATCH 11/25] fix(tokenWorker): give helper stderr to its request The persistent ctags and srcml2token helpers inherited the worker's stderr. blobExec forwards that stream to its log, so ctags warnings now reached the log for blobs that succeed. The old per-blob chain showed child stderr only when a blob failed. Each helper now writes stderr to its own temp file. After each request the worker moves the new bytes into that request's stderr, also for a helper that died during the request. A file, not a pipe: a full pipe would block the helper while the worker waits on its stdout. The driver checks that a fake ctags' stderr reaches the failed request. --- tests/token_worker_driver.py | 17 +++++++++++++--- tokenize/CregitSrcMl.pm | 38 ++++++++++++++++++++++++++++++------ 2 files changed, 46 insertions(+), 9 deletions(-) diff --git a/tests/token_worker_driver.py b/tests/token_worker_driver.py index abdb8203..c062ce70 100755 --- a/tests/token_worker_driver.py +++ b/tests/token_worker_driver.py @@ -117,10 +117,19 @@ def in_process_checks(args, marker, expected_first): encoding="utf-8", ) fake_srcml.chmod(0o755) + real_ctags = subprocess.run(["which", "ctags"], stdout=subprocess.PIPE, check=True).stdout.decode().strip() + fake_ctags = args.temp_root / "fake-ctags.sh" + fake_ctags.write_text( + "#!/usr/bin/env bash\n" + "echo HELPER_STDERR_MARKER >&2\n" + f'exec {real_ctags} "$@"\n', + encoding="utf-8", + ) + fake_ctags.chmod(0o755) memo = args.temp_root / "inproc-memo" memo.mkdir() - command = f"{args.tokenize_command} --srcml={fake_srcml}".replace( - f"--srcml={real_srcml} ", "") + options = [o for o in args.tokenize_command.split() if not o.startswith(("--srcml=", "--ctags="))] + command = " ".join(options + [f"--srcml={fake_srcml}", f"--ctags={fake_ctags}"]) worker = Worker(args.worker, memo, command) started = time.monotonic() @@ -134,7 +143,9 @@ def in_process_checks(args, marker, expected_first): crash = worker.request("crash.c", "fixtures/crash.c", b"int ABORT_MARKER;\n", 20) if crash[0] != 33 or b"killed by signal 6" not in crash[2]: raise AssertionError(f"srcml death was not reported as a parser crash: {crash!r}") - print("PASS in-process srcml crash exits 33 without output") + if b"HELPER_STDERR_MARKER" not in crash[2]: + raise AssertionError(f"helper stderr did not reach the request's stderr: {crash!r}") + print("PASS in-process srcml crash exits 33 without output, with helper stderr") if any(memo.rglob("*")): raise AssertionError("failed in-process requests were memoized") diff --git a/tokenize/CregitSrcMl.pm b/tokenize/CregitSrcMl.pm index 7bae085b..53ce499b 100644 --- a/tokenize/CregitSrcMl.pm +++ b/tokenize/CregitSrcMl.pm @@ -7,6 +7,7 @@ use strict; use warnings; use bytes; use Errno qw(EINTR); +use File::Temp (); use POSIX qw(_exit); our $PARSER_CRASH_EXIT = 33; @@ -43,12 +44,14 @@ sub tokenize { }; my $error = $@; alarm(0); + my $helperErr = $self->take_helper_stderr(); if ($self->{timedOut}) { $self->shutdown(); - return (124, "", "tokenize command timed out after [$timeoutSecs] seconds\n"); + return (124, "", $helperErr . "tokenize command timed out after [$timeoutSecs] seconds\n"); } - return (255, "", $error || "srcml tokenization failed\n") if not $ok; + return (255, "", $helperErr . ($error || "srcml tokenization failed\n")) if not $ok; + $result[2] = $helperErr . $result[2]; return @result; } @@ -199,8 +202,7 @@ sub read_declarations { sub drop_ctags { my ($self, $language) = @_; - my $proc = delete $self->{ctagsProc}{$language}; - reap_pipe($proc) if $proc; + $self->retire(delete $self->{ctagsProc}{$language}); } sub json_str { @@ -279,7 +281,7 @@ sub parse_xml { while (1) { my $line = readline($proc->{out}); if (not defined $line) { - reap_pipe(delete $self->{s2t}); + $self->retire(delete $self->{s2t}); return (undef, \@tokens, $stderr); } if ($line =~ /\A\x01END ([0-9]+)\n\z/) { @@ -302,6 +304,7 @@ sub spawn_pipe { my ($command) = @_; pipe(my $childIn, my $parentOut) or die "unable to create pipe: $!\n"; pipe(my $parentIn, my $childOut) or die "unable to create pipe: $!\n"; + my $err = File::Temp->new(TEMPLATE => "cregit-helper-err-XXXXX", TMPDIR => 1); my $pid = fork(); die "unable to fork [$command->[0]]: $!\n" if not defined $pid; @@ -310,6 +313,7 @@ sub spawn_pipe { close($parentIn); open(STDIN, '<&', $childIn) or _exit(255); open(STDOUT, '>&', $childOut) or _exit(255); + open(STDERR, '>>', $err->filename) or _exit(255); close($childIn); close($childOut); { no warnings 'exec'; exec @$command; } @@ -320,7 +324,29 @@ sub spawn_pipe { close($childOut); binmode $parentOut; binmode $parentIn; - return { pid => $pid, in => $parentOut, out => $parentIn }; + return { pid => $pid, in => $parentOut, out => $parentIn, err => $err }; +} + +# A helper's stderr goes to a file, not a pipe: a full pipe would block the +# helper while the worker waits for its stdout. +sub take_helper_stderr { + my ($self) = @_; + my @procs = grep { $_ } values(%{ $self->{ctagsProc} }), $self->{s2t}; + my $text = ""; + for my $err (@{ delete $self->{retiredErr} // [] }, map { $_->{err} } @procs) { + seek($err, 0, 0); + local $/; + $text .= <$err> // ""; + truncate($err, 0); + } + return $text; +} + +sub retire { + my ($self, $proc) = @_; + return unless $proc; + push @{ $self->{retiredErr} }, $proc->{err}; + reap_pipe($proc); } sub reap_pipe { From 9d67aa0df2ec4d05663c8fc17d600a14645c5c72 Mon Sep 17 00:00:00 2001 From: Ellian Carlos Date: Sun, 4 Oct 2026 14:13:58 +0000 Subject: [PATCH 12/25] fix(blobExec): refuse --tokenizer-worker outside pipelines Only the pipelined walkers send blobs through the worker pool. The serial walker and --shard=K/N run BlobExec.run without it, but main started the pool in every mode. So --mode serial --tokenizer-worker started one idle worker per CPU and still spawned tokenBySha.pl for each blob. blobExec now rejects the flag unless --pipeline or --pipeline-trees is given; --shard already rejects both. run_pipeline_process.sh checks the same before step 1, so a serial or sharded run fails at once. --- blobExec/src/main/scala/cregit/blobexec/Main.scala | 4 ++++ run_pipeline_process.sh | 6 +++++- 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/blobExec/src/main/scala/cregit/blobexec/Main.scala b/blobExec/src/main/scala/cregit/blobexec/Main.scala index 3b45df6b..ef162934 100644 --- a/blobExec/src/main/scala/cregit/blobexec/Main.scala +++ b/blobExec/src/main/scala/cregit/blobexec/Main.scala @@ -266,6 +266,10 @@ object Main { System.err.println(s"Error: tokenizer worker [$path] is not an executable file") sys.exit(1) } + if (!(pipeline || pipelineTrees)) { + System.err.println("Error: --tokenizer-worker needs --pipeline or --pipeline-trees") + sys.exit(1) + } } if (mask.isEmpty) { System.err.println("Error: fileMaskRegex must be non-empty") diff --git a/run_pipeline_process.sh b/run_pipeline_process.sh index 989c3f9a..8e0fe109 100755 --- a/run_pipeline_process.sh +++ b/run_pipeline_process.sh @@ -85,7 +85,8 @@ Output: --tokenizer-worker keep one tokenizeByBlobId/tokenWorker.pl process alive per blobExec worker slot instead of spawning the full wrapper - chain for every blob (default: off). + chain for every blob (default: off). Needs --mode + pipeline or pipeline-trees. --memory-limit SIZE cap the DuckDB heap in the dataset generator (step 10). Omit to accept that script's own default of 8GB. Takes an @@ -492,6 +493,9 @@ case "$MODE" in serial|pipeline|pipeline-trees|sharded) ;; *) echo "invalid --mode: $MODE (serial|pipeline|pipeline-trees|sharded)" >&2; exit 2 ;; esac +if [ "$TOKENIZER_WORKER" = 1 ] && [ "$MODE" != pipeline ] && [ "$MODE" != pipeline-trees ]; then + echo "--tokenizer-worker needs --mode pipeline or pipeline-trees" >&2; exit 2 +fi if [ "$MODE" = "sharded" ]; then [ "$SHARDS" -ge 1 ] 2>/dev/null || { echo "--shards must be a positive integer" >&2; exit 2; } fi From a4b0d8d97208b3b8ec6d31c570e1cb0e82b83e9d Mon Sep 17 00:00:00 2001 From: Ellian Carlos Date: Sun, 4 Oct 2026 14:15:09 +0000 Subject: [PATCH 13/25] refactor(blobExec): take --tokenizer-worker= Every other value flag of blobExec has the --name=value form and goes through the one flags match. --tokenizer-worker took a separate argument, so it needed its own pre-pass over args. It is now --tokenizer-worker=, parsed with the other flags; a repeated flag works as --warm= does, the last one wins. run_pipeline_process.sh passes it with the same ${VAR:+...} form as --blob-timeout, so the array block goes away. Its own --tokenizer-worker switch is unchanged. --- .../src/main/scala/cregit/blobexec/Main.scala | 26 +++++-------------- run_pipeline_process.sh | 10 +++---- 2 files changed, 9 insertions(+), 27 deletions(-) diff --git a/blobExec/src/main/scala/cregit/blobexec/Main.scala b/blobExec/src/main/scala/cregit/blobexec/Main.scala index ef162934..72737559 100644 --- a/blobExec/src/main/scala/cregit/blobexec/Main.scala +++ b/blobExec/src/main/scala/cregit/blobexec/Main.scala @@ -71,11 +71,11 @@ object Main { // `raw` (not `s`): the mask example below contains a regex backslash, which a // processed-escape interpolator rejects. `$$` therefore renders a literal `$`. private val Usage = - raw"""Usage: blobExec [--abort-on-error] [--pipeline | --pipeline-trees | --shard=K/N] [--warm=] [--tokenizer-worker ] [--blob-timeout=] [--stall-timeout=] + raw"""Usage: blobExec [--abort-on-error] [--pipeline | --pipeline-trees | --shard=K/N] [--warm=] [--tokenizer-worker=] [--blob-timeout=] [--stall-timeout=] | | --abort-on-error exit immediately (status 2) on the first non-zero | exit from , instead of skipping that blob - | --tokenizer-worker + | --tokenizer-worker= | keep one protocol-speaking tokenizer process alive per | available processor and route blob requests through it. | The worker receives BFG_MEMO_DIR and BFG_TOKENIZE_CMD @@ -141,30 +141,14 @@ object Main { |""".stripMargin def main(args: Array[String]): Unit = { - val remainingArgs = Vector.newBuilder[String] - var tokenizerWorkerPath: Option[java.nio.file.Path] = None - var argIndex = 0 - while (argIndex < args.length) { - if (args(argIndex) == "--tokenizer-worker") { - if (tokenizerWorkerPath.isDefined || argIndex + 1 >= args.length) { - System.err.println("Error: --tokenizer-worker requires exactly one path") - System.err.println(Usage) - sys.exit(1) - } - tokenizerWorkerPath = Some(Paths.get(args(argIndex + 1))) - argIndex += 2 - } else { - remainingArgs += args(argIndex) - argIndex += 1 - } - } - val (flags, positional) = remainingArgs.result().partition(_.startsWith("-")) + val (flags, positional) = args.partition(_.startsWith("-")) var abortOnError = false var pipeline = false var pipelineTrees = false var shard: Option[(Int, Int)] = None var warmPath: Option[java.nio.file.Path] = None + var tokenizerWorkerPath: Option[java.nio.file.Path] = None var blobTimeoutSeconds = BlobExec.DefaultTimeoutSeconds var stallTimeoutSeconds = Walker.DefaultStallTimeoutSeconds var stallExplicit = false @@ -194,6 +178,8 @@ object Main { sys.exit(1) } warmPath = Some(p) + case w if w.startsWith("--tokenizer-worker=") => + tokenizerWorkerPath = Some(Paths.get(w.stripPrefix("--tokenizer-worker="))) case t if t.startsWith("--blob-timeout=") => val spec = t.stripPrefix("--blob-timeout=") parsePositiveSeconds(spec) match { diff --git a/run_pipeline_process.sh b/run_pipeline_process.sh index 8e0fe109..e7201c01 100755 --- a/run_pipeline_process.sh +++ b/run_pipeline_process.sh @@ -203,7 +203,7 @@ WORK="../cregit-files" MEMO_DIR="" SKIP_HTML=0 GC_MODE="plain" -TOKENIZER_WORKER=0 +TOKENIZER_WORKER="" MEMORY_LIMIT="" DUCKDB_THREADS="" # Empty means "do not pass the flag", as above: step 10 then emits the @@ -493,7 +493,7 @@ case "$MODE" in serial|pipeline|pipeline-trees|sharded) ;; *) echo "invalid --mode: $MODE (serial|pipeline|pipeline-trees|sharded)" >&2; exit 2 ;; esac -if [ "$TOKENIZER_WORKER" = 1 ] && [ "$MODE" != pipeline ] && [ "$MODE" != pipeline-trees ]; then +if [ -n "$TOKENIZER_WORKER" ] && [ "$MODE" != pipeline ] && [ "$MODE" != pipeline-trees ]; then echo "--tokenizer-worker needs --mode pipeline or pipeline-trees" >&2; exit 2 fi if [ "$MODE" = "sharded" ]; then @@ -753,12 +753,8 @@ else MODE_FLAG="" [ "$MODE" = "pipeline" ] && MODE_FLAG="--pipeline" [ "$MODE" = "pipeline-trees" ] && MODE_FLAG="--pipeline-trees" - TOKENIZER_WORKER_FLAGS=() - if [ "$TOKENIZER_WORKER" = 1 ]; then - TOKENIZER_WORKER_FLAGS=("--tokenizer-worker" "${CREGIT}/tokenizeByBlobId/tokenWorker.pl") - fi java -jar "$BFG" $MODE_FLAG \ - ${TOKENIZER_WORKER_FLAGS[@]+"${TOKENIZER_WORKER_FLAGS[@]}"} \ + ${TOKENIZER_WORKER:+"--tokenizer-worker=${CREGIT}/tokenizeByBlobId/tokenWorker.pl"} \ ${BLOB_TIMEOUT:+--blob-timeout=$BLOB_TIMEOUT} \ ${STALL_TIMEOUT:+--stall-timeout=$STALL_TIMEOUT} \ "$REPO_PATH_ORIGINAL_BARE" \ From 1b4d214e72ba8942fa6cbe985c67a55d0124aa3a Mon Sep 17 00:00:00 2001 From: Ellian Carlos Date: Sun, 4 Oct 2026 14:16:38 +0000 Subject: [PATCH 14/25] refactor(blobExec): pass the budget to each worker request The pool took one timeout in its constructor, and Walker ran the pool before BlobExec.run and handed it the finished result. A caller that retries a timed-out blob with a larger budget (#96) could not use the pool: the budget was fixed, and a finished result cannot be re-run. The REQ header already carries a timeout per request. invoke now takes timeoutSeconds, and BlobExec.run takes the pool and calls it with its own timeoutSeconds, as it calls ChildRunner. Walker only passes the pool through. A new test checks that each request sends its own budget. --- .../main/scala/cregit/blobexec/BlobExec.scala | 8 +++- .../src/main/scala/cregit/blobexec/Main.scala | 2 +- .../cregit/blobexec/TokenizerWorkerPool.scala | 13 +++--- .../main/scala/cregit/blobexec/Walker.scala | 4 +- .../blobexec/TokenizerWorkerPoolSpec.scala | 46 +++++++++++++------ 5 files changed, 47 insertions(+), 26 deletions(-) diff --git a/blobExec/src/main/scala/cregit/blobexec/BlobExec.scala b/blobExec/src/main/scala/cregit/blobexec/BlobExec.scala index f5f8ee33..3690dbf6 100644 --- a/blobExec/src/main/scala/cregit/blobexec/BlobExec.scala +++ b/blobExec/src/main/scala/cregit/blobexec/BlobExec.scala @@ -47,11 +47,15 @@ object BlobExec { timeoutSeconds: Int = DefaultTimeoutSeconds, onTimeout: () => Unit = () => (), onParserCrash: () => Unit = () => (), - invocationResult: Option[ChildRunner.Outcome] = None + workerPool: Option[TokenizerWorkerPool] = None ): Outcome = { val env = Seq("BFG_BLOB" -> origSha, "BFG_FILENAME" -> filename, "BFG_PATH" -> fullPath) - invocationResult.getOrElse(new ChildRunner(timeoutSeconds).run(command, bytes, env)) match { + val ran = workerPool match { + case Some(pool) => pool.invoke(bytes, origSha, filename, fullPath, timeoutSeconds) + case None => new ChildRunner(timeoutSeconds).run(command, bytes, env) + } + ran match { case ChildRunner.Outcome.Killed(why) => System.err.println( s"Warning: command [$command] on blob $origSha at path [$fullPath] gave no usable " + diff --git a/blobExec/src/main/scala/cregit/blobexec/Main.scala b/blobExec/src/main/scala/cregit/blobexec/Main.scala index 72737559..3f236612 100644 --- a/blobExec/src/main/scala/cregit/blobexec/Main.scala +++ b/blobExec/src/main/scala/cregit/blobexec/Main.scala @@ -300,7 +300,7 @@ object Main { val workerEnv = Seq("BFG_MEMO_DIR", "BFG_TOKENIZE_CMD").flatMap { key => sys.env.get(key).map(key -> _) }.toMap - new TokenizerWorkerPool(Seq(path.toString), workerEnv, parallelism, blobTimeoutSeconds) + new TokenizerWorkerPool(Seq(path.toString), workerEnv, parallelism) } workerShutdownHook = workerPool.map { pool => val hook = new Thread(() => pool.close(), "tokenizer-worker-pool-shutdown") diff --git a/blobExec/src/main/scala/cregit/blobexec/TokenizerWorkerPool.scala b/blobExec/src/main/scala/cregit/blobexec/TokenizerWorkerPool.scala index 6786b63e..9915ef46 100644 --- a/blobExec/src/main/scala/cregit/blobexec/TokenizerWorkerPool.scala +++ b/blobExec/src/main/scala/cregit/blobexec/TokenizerWorkerPool.scala @@ -9,13 +9,11 @@ import scala.jdk.CollectionConverters._ final class TokenizerWorkerPool( workerCommand: Seq[String], env: Map[String, String], - size: Int, - timeoutSeconds: Int + size: Int ) extends AutoCloseable { require(workerCommand.nonEmpty, "workerCommand must not be empty") require(size > 0, "worker pool size must be positive") - require(timeoutSeconds > 0, "worker timeout must be positive") private val WorkerTimeoutExitCode = 124 @@ -58,14 +56,16 @@ final class TokenizerWorkerPool( bytes: Array[Byte], origSha: String, filename: String, - fullPath: String + fullPath: String, + timeoutSeconds: Int ): ChildRunner.Outcome = { + require(timeoutSeconds > 0, "worker timeout must be positive") if (closed.get()) throw new IllegalStateException("tokenizer worker pool is closed") val worker = idle.take() worker.takeDiagnostics() try { - writeRequest(worker, bytes, origSha, filename, fullPath) + writeRequest(worker, bytes, origSha, filename, fullPath, timeoutSeconds) } catch { case failure: IOException => return failedWorker(worker, failure) @@ -188,7 +188,8 @@ final class TokenizerWorkerPool( bytes: Array[Byte], origSha: String, filename: String, - fullPath: String + fullPath: String, + timeoutSeconds: Int ): Unit = { val filenameBytes = filename.getBytes(UTF_8) val pathBytes = fullPath.getBytes(UTF_8) diff --git a/blobExec/src/main/scala/cregit/blobexec/Walker.scala b/blobExec/src/main/scala/cregit/blobexec/Walker.scala index 0a3211f3..46e01538 100644 --- a/blobExec/src/main/scala/cregit/blobexec/Walker.scala +++ b/blobExec/src/main/scala/cregit/blobexec/Walker.scala @@ -843,8 +843,6 @@ final class Walker( try { blobCommandExecutions.increment() val unusable = new AtomicBoolean(false) - val invocationResult = - workerPool.map(_.invoke(bytes, task.origId.name, task.filename, task.fullPath)) val outcome = BlobExec.run( bytes = bytes, origSha = task.origId.name, @@ -856,7 +854,7 @@ final class Walker( timeoutSeconds = blobTimeoutSeconds, onTimeout = () => { blobsTimedOut.increment(); unusable.set(true) }, onParserCrash = () => { blobsParserCrashed.increment(); unusable.set(true) }, - invocationResult = invocationResult + workerPool = workerPool ) val res = outcome match { case BlobExec.Outcome.Skip if unusable.get() => diff --git a/blobExec/src/test/scala/cregit/blobexec/TokenizerWorkerPoolSpec.scala b/blobExec/src/test/scala/cregit/blobexec/TokenizerWorkerPoolSpec.scala index ba0b6a6a..6a937646 100644 --- a/blobExec/src/test/scala/cregit/blobexec/TokenizerWorkerPoolSpec.scala +++ b/blobExec/src/test/scala/cregit/blobexec/TokenizerWorkerPoolSpec.scala @@ -18,6 +18,15 @@ class TokenizerWorkerPoolSpec extends AnyFunSuite with Matchers with BeforeAndAf case other => fail(s"expected Exited, got $other") } + private def request( + pool: TokenizerWorkerPool, + filename: String, + body: String, + sha: String, + timeoutSeconds: Int = 2 + ): ChildRunner.Outcome = + pool.invoke(body.getBytes(UTF_8), sha, filename, s"src/$filename", timeoutSeconds) + private val tmpDir = Files.createTempDirectory("tokenizer-worker-pool-") private val worker = tmpDir.resolve("fake-worker.pl") @@ -62,6 +71,8 @@ class TokenizerWorkerPoolSpec extends AnyFunSuite with Matchers with BeforeAndAf | print "RES 33 0 5\ncrash"; | } elsif ($filename eq 'timeout.c') { | print "RES 124 0 7\ntimeout"; + | } elsif ($filename eq 'budget.c') { + | print "RES 0 ", length($timeout), " 0\n", $timeout; | } elsif ($filename eq 'die.c') { | print STDERR "worker died\n"; | exit 7; @@ -78,33 +89,40 @@ class TokenizerWorkerPoolSpec extends AnyFunSuite with Matchers with BeforeAndAf override def afterAll(): Unit = deleteRecursive(tmpDir) test("round-trips bytes and preserves parser crash status") { - val pool = new TokenizerWorkerPool(Seq(worker.toString), Map.empty, size = 1, timeoutSeconds = 2) + val pool = new TokenizerWorkerPool(Seq(worker.toString), Map.empty, size = 1) try { - val (exit, stdout, stderr) = exited(pool.invoke( - "hello".getBytes(UTF_8), "a" * 40, "hello.c", "src/hello.c")) + val (exit, stdout, stderr) = exited(request(pool, "hello.c", "hello", "a" * 40)) exit shouldEqual 0 stdout shouldEqual "HELLO".getBytes(UTF_8) stderr shouldBe empty val (crashExit, crashOut, crashErr) = - exited(pool.invoke("bad".getBytes(UTF_8), "b" * 40, "crash.c", "src/crash.c")) + exited(request(pool, "crash.c", "bad", "b" * 40)) crashExit shouldEqual BlobExec.ParserCrashExitCode crashOut shouldBe empty crashErr shouldEqual "crash" } finally pool.close() } + test("sends each request's own budget to the worker") { + val pool = new TokenizerWorkerPool(Seq(worker.toString), Map.empty, size = 1) + try { + exited(request(pool, "budget.c", "x", "9" * 40, timeoutSeconds = 7))._2 shouldEqual "7".getBytes(UTF_8) + exited(request(pool, "budget.c", "x", "9" * 40, timeoutSeconds = 21))._2 shouldEqual "21".getBytes(UTF_8) + } finally pool.close() + } + test("times out a wedged worker, replaces it, and serves the next request") { - val pool = new TokenizerWorkerPool(Seq(worker.toString), Map.empty, size = 1, timeoutSeconds = 1) + val pool = new TokenizerWorkerPool(Seq(worker.toString), Map.empty, size = 1) try { val started = System.nanoTime() - val hung = pool.invoke("hang".getBytes(UTF_8), "c" * 40, "hang.c", "src/hang.c") + val hung = request(pool, "hang.c", "hang", "c" * 40, timeoutSeconds = 1) val elapsed = (System.nanoTime() - started).nanos hung shouldBe a[Killed] elapsed should be < 15.seconds val (nextExit, nextOut, nextErr) = - exited(pool.invoke("next".getBytes(UTF_8), "d" * 40, "next.c", "src/next.c")) + exited(request(pool, "next.c", "next", "d" * 40, timeoutSeconds = 1)) nextExit shouldEqual 0 nextOut shouldEqual "NEXT".getBytes(UTF_8) nextErr shouldBe empty @@ -113,11 +131,11 @@ class TokenizerWorkerPoolSpec extends AnyFunSuite with Matchers with BeforeAndAf test("serves pool-sized requests in parallel") { implicit val ec: ExecutionContext = ExecutionContext.global - val pool = new TokenizerWorkerPool(Seq(worker.toString), Map.empty, size = 3, timeoutSeconds = 5) + val pool = new TokenizerWorkerPool(Seq(worker.toString), Map.empty, size = 3) try { val started = System.nanoTime() val requests = (1 to 3).map { n => - Future(exited(pool.invoke(s"body$n".getBytes(UTF_8), f"$n%040x", "slow.c", s"src/slow$n.c"))) + Future(exited(request(pool, "slow.c", s"body$n", f"$n%040x", timeoutSeconds = 5))) } val results = Await.result(Future.sequence(requests), 10.seconds) val elapsed = (System.nanoTime() - started).nanos @@ -129,24 +147,24 @@ class TokenizerWorkerPoolSpec extends AnyFunSuite with Matchers with BeforeAndAf } test("maps worker timeout status and respawns after mid-request death") { - val pool = new TokenizerWorkerPool(Seq(worker.toString), Map.empty, size = 1, timeoutSeconds = 2) + val pool = new TokenizerWorkerPool(Seq(worker.toString), Map.empty, size = 1) try { - val timedOut = pool.invoke("slow".getBytes(UTF_8), "e" * 40, "timeout.c", "src/timeout.c") + val timedOut = request(pool, "timeout.c", "slow", "e" * 40) timedOut shouldBe a[Killed] - val died = exited(pool.invoke("die".getBytes(UTF_8), "f" * 40, "die.c", "src/die.c")) + val died = exited(request(pool, "die.c", "die", "f" * 40)) died._1 should not equal 0 died._2 shouldBe empty died._3 should include("worker died") - val recovered = exited(pool.invoke("ok".getBytes(UTF_8), "1" * 40, "ok.c", "src/ok.c")) + val recovered = exited(request(pool, "ok.c", "ok", "1" * 40)) recovered._1 shouldEqual 0 recovered._2 shouldEqual "OK".getBytes(UTF_8) } finally pool.close() } test("close terminates every worker process") { - val pool = new TokenizerWorkerPool(Seq(worker.toString), Map.empty, size = 2, timeoutSeconds = 2) + val pool = new TokenizerWorkerPool(Seq(worker.toString), Map.empty, size = 2) val pids = pool.currentWorkerPids pids should have size 2 From 4404bd524adaee235243aee263b783de646d2b95 Mon Sep 17 00:00:00 2001 From: Ellian Carlos Date: Sun, 4 Oct 2026 14:18:23 +0000 Subject: [PATCH 15/25] refactor(blobExec): reuse ChildRunner's lifetime and kill The pool computed its backstop as budget + 5 + 5, which is ChildRunner.maxLifetimeSeconds written out again. It replaced a worker with destroyForcibly, which kills only the perl process and leaves its children, such as a wedged srcml in its own process group. It now calls ChildRunner.killTree, as the per-blob path does. replaceWorker had a force parameter that every caller set to true; it is gone. The wedged-worker test now also checks that the worker's child dies. --- .../scala/cregit/blobexec/TokenizerWorkerPool.scala | 12 ++++++------ .../cregit/blobexec/TokenizerWorkerPoolSpec.scala | 11 +++++++++-- 2 files changed, 15 insertions(+), 8 deletions(-) diff --git a/blobExec/src/main/scala/cregit/blobexec/TokenizerWorkerPool.scala b/blobExec/src/main/scala/cregit/blobexec/TokenizerWorkerPool.scala index 9915ef46..5b2434ff 100644 --- a/blobExec/src/main/scala/cregit/blobexec/TokenizerWorkerPool.scala +++ b/blobExec/src/main/scala/cregit/blobexec/TokenizerWorkerPool.scala @@ -84,9 +84,9 @@ final class TokenizerWorkerPool( reader.setDaemon(true) reader.start() - val backstopSeconds = math.max(1, timeoutSeconds).toLong + 5L + 5L + val backstopSeconds = ChildRunner.maxLifetimeSeconds(timeoutSeconds) if (!finished.await(backstopSeconds, TimeUnit.SECONDS)) { - replaceWorker(worker, force = true) + replaceWorker(worker) ChildRunner.Outcome.Killed(s"no response within ${backstopSeconds}s; worker replaced") } else { response.get() match { @@ -250,7 +250,7 @@ final class TokenizerWorkerPool( private def failedWorker(worker: Worker, failure: Throwable): ChildRunner.Outcome = { val workerExit = if (worker.process.waitFor(100, TimeUnit.MILLISECONDS)) Some(worker.process.exitValue()) else None val diagnostics = worker.takeDiagnostics() - replaceWorker(worker, force = true) + replaceWorker(worker) if (workerExit.contains(WorkerTimeoutExitCode)) ChildRunner.Outcome.Killed(s"worker exited $WorkerTimeoutExitCode (timeout)") else { @@ -261,13 +261,13 @@ final class TokenizerWorkerPool( private def returnWorker(worker: Worker): Unit = { if (worker.process.isAlive && !closed.get()) idle.put(worker) - else replaceWorker(worker, force = true) + else replaceWorker(worker) } - private def replaceWorker(worker: Worker, force: Boolean): Unit = { + private def replaceWorker(worker: Worker): Unit = { workers.remove(worker) try worker.stdin.close() catch { case _: IOException => () } - if (force && worker.process.isAlive) worker.process.destroyForcibly() + if (worker.process.isAlive) ChildRunner.killTree(worker.process) if (!closed.get()) { val replacement = startWorker(worker.index) workers.add(replacement) diff --git a/blobExec/src/test/scala/cregit/blobexec/TokenizerWorkerPoolSpec.scala b/blobExec/src/test/scala/cregit/blobexec/TokenizerWorkerPoolSpec.scala index 6a937646..fc598373 100644 --- a/blobExec/src/test/scala/cregit/blobexec/TokenizerWorkerPoolSpec.scala +++ b/blobExec/src/test/scala/cregit/blobexec/TokenizerWorkerPoolSpec.scala @@ -5,7 +5,7 @@ import org.scalatest.funsuite.AnyFunSuite import org.scalatest.matchers.should.Matchers import java.nio.charset.StandardCharsets.UTF_8 -import java.nio.file.{Files, Path} +import java.nio.file.{Files, Path, Paths} import scala.concurrent.duration._ import scala.concurrent.{Await, ExecutionContext, Future} @@ -62,6 +62,11 @@ class TokenizerWorkerPoolSpec extends AnyFunSuite with Matchers with BeforeAndAf | my $body = read_exact($body_len); | | if ($filename eq 'hang.c') { + | my $child = fork() // exit 3; + | exec 'sleep', '300' if $child == 0; + | open(my $pidFile, '>', "$0.grandchild") or exit 3; + | print {$pidFile} $child; + | close($pidFile); | sleep 300 while 1; | } elsif ($filename eq 'slow.c') { | sleep 2; @@ -112,7 +117,7 @@ class TokenizerWorkerPoolSpec extends AnyFunSuite with Matchers with BeforeAndAf } finally pool.close() } - test("times out a wedged worker, replaces it, and serves the next request") { + test("times out a wedged worker, kills its children, replaces it, and serves the next request") { val pool = new TokenizerWorkerPool(Seq(worker.toString), Map.empty, size = 1) try { val started = System.nanoTime() @@ -121,6 +126,8 @@ class TokenizerWorkerPoolSpec extends AnyFunSuite with Matchers with BeforeAndAf hung shouldBe a[Killed] elapsed should be < 15.seconds + val grandchild = Files.readString(Paths.get(s"$worker.grandchild")).trim.toLong + eventuallyStopped(Seq(grandchild)) shouldBe true val (nextExit, nextOut, nextErr) = exited(request(pool, "next.c", "next", "d" * 40, timeoutSeconds = 1)) nextExit shouldEqual 0 From 8c9c56323fcacceabc9508baf2b49df0ff682357 Mon Sep 17 00:00:00 2001 From: Ellian Carlos Date: Sun, 4 Oct 2026 14:19:30 +0000 Subject: [PATCH 16/25] refactor(blobExec): drop the worker env map main copied BFG_MEMO_DIR and BFG_TOKENIZE_CMD from sys.env into the worker's ProcessBuilder, which already starts with a copy of the JVM's environment. The env parameter of TokenizerWorkerPool did nothing and every test passed Map.empty; it is gone. The usage text said the worker receives only those two variables. It now says that the worker inherits the environment, and that the flag needs a pipelined mode. --- blobExec/src/main/scala/cregit/blobexec/Main.scala | 11 ++++------- .../scala/cregit/blobexec/TokenizerWorkerPool.scala | 2 -- .../cregit/blobexec/TokenizerWorkerPoolSpec.scala | 12 ++++++------ 3 files changed, 10 insertions(+), 15 deletions(-) diff --git a/blobExec/src/main/scala/cregit/blobexec/Main.scala b/blobExec/src/main/scala/cregit/blobexec/Main.scala index 3f236612..97ee91b0 100644 --- a/blobExec/src/main/scala/cregit/blobexec/Main.scala +++ b/blobExec/src/main/scala/cregit/blobexec/Main.scala @@ -78,9 +78,9 @@ object Main { | --tokenizer-worker= | keep one protocol-speaking tokenizer process alive per | available processor and route blob requests through it. - | The worker receives BFG_MEMO_DIR and BFG_TOKENIZE_CMD - | from this process; without this flag is run - | once per blob as before. + | The worker inherits this process's environment. Needs + | --pipeline or --pipeline-trees; without this flag + | is run once per blob. | --blob-timeout= | budget for one invocation (default ${BlobExec.DefaultTimeoutSeconds}). | A child that exceeds it is killed with everything @@ -297,10 +297,7 @@ object Main { val (stats, timedOutEver) = try { val parallelism = math.max(1, Runtime.getRuntime.availableProcessors) workerPool = tokenizerWorkerPath.map { path => - val workerEnv = Seq("BFG_MEMO_DIR", "BFG_TOKENIZE_CMD").flatMap { key => - sys.env.get(key).map(key -> _) - }.toMap - new TokenizerWorkerPool(Seq(path.toString), workerEnv, parallelism) + new TokenizerWorkerPool(Seq(path.toString), parallelism) } workerShutdownHook = workerPool.map { pool => val hook = new Thread(() => pool.close(), "tokenizer-worker-pool-shutdown") diff --git a/blobExec/src/main/scala/cregit/blobexec/TokenizerWorkerPool.scala b/blobExec/src/main/scala/cregit/blobexec/TokenizerWorkerPool.scala index 5b2434ff..eb404726 100644 --- a/blobExec/src/main/scala/cregit/blobexec/TokenizerWorkerPool.scala +++ b/blobExec/src/main/scala/cregit/blobexec/TokenizerWorkerPool.scala @@ -8,7 +8,6 @@ import scala.jdk.CollectionConverters._ final class TokenizerWorkerPool( workerCommand: Seq[String], - env: Map[String, String], size: Int ) extends AutoCloseable { @@ -122,7 +121,6 @@ final class TokenizerWorkerPool( private def startWorker(index: Int): Worker = { val builder = new ProcessBuilder(workerCommand: _*) - builder.environment().putAll(env.asJava) val process = builder.start() val worker = new Worker( index, diff --git a/blobExec/src/test/scala/cregit/blobexec/TokenizerWorkerPoolSpec.scala b/blobExec/src/test/scala/cregit/blobexec/TokenizerWorkerPoolSpec.scala index fc598373..2fbb9db3 100644 --- a/blobExec/src/test/scala/cregit/blobexec/TokenizerWorkerPoolSpec.scala +++ b/blobExec/src/test/scala/cregit/blobexec/TokenizerWorkerPoolSpec.scala @@ -94,7 +94,7 @@ class TokenizerWorkerPoolSpec extends AnyFunSuite with Matchers with BeforeAndAf override def afterAll(): Unit = deleteRecursive(tmpDir) test("round-trips bytes and preserves parser crash status") { - val pool = new TokenizerWorkerPool(Seq(worker.toString), Map.empty, size = 1) + val pool = new TokenizerWorkerPool(Seq(worker.toString), size = 1) try { val (exit, stdout, stderr) = exited(request(pool, "hello.c", "hello", "a" * 40)) exit shouldEqual 0 @@ -110,7 +110,7 @@ class TokenizerWorkerPoolSpec extends AnyFunSuite with Matchers with BeforeAndAf } test("sends each request's own budget to the worker") { - val pool = new TokenizerWorkerPool(Seq(worker.toString), Map.empty, size = 1) + val pool = new TokenizerWorkerPool(Seq(worker.toString), size = 1) try { exited(request(pool, "budget.c", "x", "9" * 40, timeoutSeconds = 7))._2 shouldEqual "7".getBytes(UTF_8) exited(request(pool, "budget.c", "x", "9" * 40, timeoutSeconds = 21))._2 shouldEqual "21".getBytes(UTF_8) @@ -118,7 +118,7 @@ class TokenizerWorkerPoolSpec extends AnyFunSuite with Matchers with BeforeAndAf } test("times out a wedged worker, kills its children, replaces it, and serves the next request") { - val pool = new TokenizerWorkerPool(Seq(worker.toString), Map.empty, size = 1) + val pool = new TokenizerWorkerPool(Seq(worker.toString), size = 1) try { val started = System.nanoTime() val hung = request(pool, "hang.c", "hang", "c" * 40, timeoutSeconds = 1) @@ -138,7 +138,7 @@ class TokenizerWorkerPoolSpec extends AnyFunSuite with Matchers with BeforeAndAf test("serves pool-sized requests in parallel") { implicit val ec: ExecutionContext = ExecutionContext.global - val pool = new TokenizerWorkerPool(Seq(worker.toString), Map.empty, size = 3) + val pool = new TokenizerWorkerPool(Seq(worker.toString), size = 3) try { val started = System.nanoTime() val requests = (1 to 3).map { n => @@ -154,7 +154,7 @@ class TokenizerWorkerPoolSpec extends AnyFunSuite with Matchers with BeforeAndAf } test("maps worker timeout status and respawns after mid-request death") { - val pool = new TokenizerWorkerPool(Seq(worker.toString), Map.empty, size = 1) + val pool = new TokenizerWorkerPool(Seq(worker.toString), size = 1) try { val timedOut = request(pool, "timeout.c", "slow", "e" * 40) timedOut shouldBe a[Killed] @@ -171,7 +171,7 @@ class TokenizerWorkerPoolSpec extends AnyFunSuite with Matchers with BeforeAndAf } test("close terminates every worker process") { - val pool = new TokenizerWorkerPool(Seq(worker.toString), Map.empty, size = 2) + val pool = new TokenizerWorkerPool(Seq(worker.toString), size = 2) val pids = pool.currentWorkerPids pids should have size 2 From e64608cc97bb8ec37dd8d2c6df9c3ca79a0bcd48 Mon Sep 17 00:00:00 2001 From: Ellian Carlos Date: Sun, 4 Oct 2026 14:20:17 +0000 Subject: [PATCH 17/25] refactor(blobExec): drop the worker pool shutdown hook The finally block of main already closes the pool. Walker never calls sys.exit, and on a stall it halts the JVM, so shutdown hooks do not run and killDescendants kills the workers. If the JVM dies in another way, the pipes close: tokenWorker.pl reads EOF on stdin, stops its ctags and srcml2token helpers and exits 0. Checked by hand: a worker that had served one request exited 0 on EOF and left no helper alive. A worker in the middle of a request stops after its own alarm. --- blobExec/src/main/scala/cregit/blobexec/Main.scala | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/blobExec/src/main/scala/cregit/blobexec/Main.scala b/blobExec/src/main/scala/cregit/blobexec/Main.scala index 97ee91b0..9d872643 100644 --- a/blobExec/src/main/scala/cregit/blobexec/Main.scala +++ b/blobExec/src/main/scala/cregit/blobexec/Main.scala @@ -293,17 +293,11 @@ object Main { // The cumulative figure is forensic only: gating on it would make one past // timeout permanent. var workerPool: Option[TokenizerWorkerPool] = None - var workerShutdownHook: Option[Thread] = None val (stats, timedOutEver) = try { val parallelism = math.max(1, Runtime.getRuntime.availableProcessors) workerPool = tokenizerWorkerPath.map { path => new TokenizerWorkerPool(Seq(path.toString), parallelism) } - workerShutdownHook = workerPool.map { pool => - val hook = new Thread(() => pool.close(), "tokenizer-worker-pool-shutdown") - Runtime.getRuntime.addShutdownHook(hook) - hook - } val walker = new Walker( src, dst, mapping, mask.r, command, abortOnError, parallelism, pipeline, pipelineTrees, shard, @@ -320,10 +314,6 @@ object Main { (s, total) } finally { workerPool.foreach(_.close()) - workerShutdownHook.foreach { hook => - try Runtime.getRuntime.removeShutdownHook(hook) - catch { case _: IllegalStateException => () } - } mapping.close() dst.close() src.close() From 16506b585be3ea0b7ab451ec68b02cab3d7f4118 Mon Sep 17 00:00:00 2001 From: Ellian Carlos Date: Sun, 4 Oct 2026 14:20:41 +0000 Subject: [PATCH 18/25] refactor(tokenWorker): drop the BFG_WORKER_TRACE trace d60935a added an optional per-request trace file. It is a profiling aid, not part of the worker: no test, script or document uses it, and the measurements in the PR do not need it. Keep it on a local branch if a later profile needs it. --- tokenizeByBlobId/tokenWorker.pl | 13 ------------- 1 file changed, 13 deletions(-) diff --git a/tokenizeByBlobId/tokenWorker.pl b/tokenizeByBlobId/tokenWorker.pl index a5e37253..4bfaca8f 100755 --- a/tokenizeByBlobId/tokenWorker.pl +++ b/tokenizeByBlobId/tokenWorker.pl @@ -11,7 +11,6 @@ use FindBin qw($RealBin); use IO::Select; use POSIX qw(_exit); -use Time::HiRes (); use File::Basename qw(basename dirname); use lib "$RealBin/../tokenize"; use CregitLanguages; @@ -36,12 +35,6 @@ my $inProcess = in_process_tokenizer(@tokenizeCommand); -my $trace; -if (my $tracePath = $ENV{BFG_WORKER_TRACE}) { - open($trace, '>>', "$tracePath.$$") or die "unable to open trace [$tracePath.$$]: $!\n"; - $trace->autoflush(1); -} - print STDOUT "READY\n"; while (defined(my $header = )) { @@ -55,7 +48,6 @@ my $contents = read_exact($bodyLen, "body"); my ($exitCode, $stdout, $stderr); - my $t0 = $trace ? Time::HiRes::time() : 0; my $ok = eval { ($exitCode, $stdout, $stderr) = process_request( $origSha, $filename, $fullPath, $contents, $timeoutSecs @@ -68,11 +60,6 @@ $stderr = $@ || "tokenizer request failed\n"; } - if ($trace) { - my $ext = $filename =~ /\.([^.]+)\z/ ? lc($1) : ""; - printf {$trace} "%s %d %s %.3f %d %d\n", - $origSha, $bodyLen, $ext, (Time::HiRes::time() - $t0) * 1000, $exitCode, length($stdout); - } print STDOUT "RES $exitCode " . length($stdout) . " " . length($stderr) . "\n"; print STDOUT $stdout, $stderr; } From b3a7d4c946aed1c3ead22ab3829d43f204b3ec0b Mon Sep 17 00:00:00 2001 From: Ellian Carlos Date: Sun, 4 Oct 2026 14:21:33 +0000 Subject: [PATCH 19/25] test: drop test-only hooks from the worker code TokenizerWorkerPool.currentWorkerPids existed only for the close test, in a shipped class. The test now finds the worker processes among the JVM's children with ProcessHandle. token_worker_driver.py had an --in-process option, and its one caller, test_token_worker.sh, always set it. The in-process checks now always run, and the option is gone. --- .../main/scala/cregit/blobexec/TokenizerWorkerPool.scala | 3 --- .../scala/cregit/blobexec/TokenizerWorkerPoolSpec.scala | 9 ++++++++- tests/test_token_worker.sh | 1 - tests/token_worker_driver.py | 4 +--- 4 files changed, 9 insertions(+), 8 deletions(-) diff --git a/blobExec/src/main/scala/cregit/blobexec/TokenizerWorkerPool.scala b/blobExec/src/main/scala/cregit/blobexec/TokenizerWorkerPool.scala index eb404726..aa03e69c 100644 --- a/blobExec/src/main/scala/cregit/blobexec/TokenizerWorkerPool.scala +++ b/blobExec/src/main/scala/cregit/blobexec/TokenizerWorkerPool.scala @@ -100,9 +100,6 @@ final class TokenizerWorkerPool( } } - private[blobexec] def currentWorkerPids: Seq[Long] = - workers.asScala.toVector.map(_.process.pid()) - override def close(): Unit = { if (closed.compareAndSet(false, true)) { val snapshot = workers.asScala.toVector diff --git a/blobExec/src/test/scala/cregit/blobexec/TokenizerWorkerPoolSpec.scala b/blobExec/src/test/scala/cregit/blobexec/TokenizerWorkerPoolSpec.scala index 2fbb9db3..c14a1913 100644 --- a/blobExec/src/test/scala/cregit/blobexec/TokenizerWorkerPoolSpec.scala +++ b/blobExec/src/test/scala/cregit/blobexec/TokenizerWorkerPoolSpec.scala @@ -8,6 +8,7 @@ import java.nio.charset.StandardCharsets.UTF_8 import java.nio.file.{Files, Path, Paths} import scala.concurrent.duration._ import scala.concurrent.{Await, ExecutionContext, Future} +import scala.jdk.CollectionConverters._ class TokenizerWorkerPoolSpec extends AnyFunSuite with Matchers with BeforeAndAfterAll { @@ -172,7 +173,7 @@ class TokenizerWorkerPoolSpec extends AnyFunSuite with Matchers with BeforeAndAf test("close terminates every worker process") { val pool = new TokenizerWorkerPool(Seq(worker.toString), size = 2) - val pids = pool.currentWorkerPids + val pids = workerPids() pids should have size 2 pool.close() @@ -180,6 +181,12 @@ class TokenizerWorkerPoolSpec extends AnyFunSuite with Matchers with BeforeAndAf eventuallyStopped(pids) shouldBe true } + private def workerPids(): Seq[Long] = + ProcessHandle.current().children().iterator().asScala + .filter(_.info().commandLine().orElse("").contains(worker.toString)) + .map(_.pid()) + .toVector + private def eventuallyStopped(pids: Seq[Long]): Boolean = { val deadline = System.nanoTime() + 5.seconds.toNanos while (System.nanoTime() < deadline && pids.exists(isAlive)) Thread.sleep(25) diff --git a/tests/test_token_worker.sh b/tests/test_token_worker.sh index 37868ecf..251f88dd 100755 --- a/tests/test_token_worker.sh +++ b/tests/test_token_worker.sh @@ -24,5 +24,4 @@ python3 "$ROOT/tests/token_worker_driver.py" \ --fixture "$ROOT/tests/fixtures/Tiny.java" \ --fixture "$ROOT/tests/fixtures/empty.c" \ --fixture "$ROOT/tests/fixtures/upper.H" \ - --in-process \ --temp-root "$TEMP_ROOT" diff --git a/tests/token_worker_driver.py b/tests/token_worker_driver.py index c062ce70..b26331e0 100755 --- a/tests/token_worker_driver.py +++ b/tests/token_worker_driver.py @@ -164,7 +164,6 @@ def main(): parser.add_argument("--tokenize-command", required=True) parser.add_argument("--fixture", action="append", required=True, type=pathlib.Path) parser.add_argument("--temp-root", required=True, type=pathlib.Path) - parser.add_argument("--in-process", action="store_true") args = parser.parse_args() worker_memo = args.temp_root / "worker-memo" @@ -242,8 +241,7 @@ def main(): timeout_worker.close() print(f"PASS timeout exits 124 in {elapsed:.2f}s with no child left") - if args.in_process: - in_process_checks(args, marker, expected_outputs[0]) + in_process_checks(args, marker, expected_outputs[0]) print("ALL TOKEN WORKER TESTS PASSED") From 79a4786be88496729e07659a1b1f019e58394ac5 Mon Sep 17 00:00:00 2001 From: Ellian Carlos Date: Sun, 4 Oct 2026 14:22:11 +0000 Subject: [PATCH 20/25] docs(tokenWorker): bring WORKER_PROTOCOL.md up to date The document said workers hold no cross-request state, and it called the in-process srcML chain "Stage B (later)", though this branch already runs it with persistent ctags and srcml2token helpers. It gave the old chain in the wrong order, a backstop of "timeoutSecs + grace + slack" where the code uses ChildRunner.maxLifetimeSeconds, and no environment variables beyond the two required ones. Describe the persistent helpers, where their stderr goes, the --tokenizer-worker= flag, the backstop and BFG_WORKER_INPROC. --- tokenizeByBlobId/WORKER_PROTOCOL.md | 40 +++++++++++++++++------------ 1 file changed, 23 insertions(+), 17 deletions(-) diff --git a/tokenizeByBlobId/WORKER_PROTOCOL.md b/tokenizeByBlobId/WORKER_PROTOCOL.md index 267e0149..ddcfc70a 100644 --- a/tokenizeByBlobId/WORKER_PROTOCOL.md +++ b/tokenizeByBlobId/WORKER_PROTOCOL.md @@ -1,9 +1,10 @@ # Persistent tokenizer worker protocol `tokenizeByBlobId/tokenWorker.pl` replaces the per-blob chain -`timeout → sh → tokenize.pl → tokenBySha.pl → tokenizeSrcMl.pl` with one -long-lived perl process per pool slot. blobExec starts `parallelism` workers and -talks to each over its stdin/stdout. One request in flight per worker. +`tokenBySha.pl → sh → tokenize.pl → tokenizeSrcMl.pl` with one long-lived perl +process per pool slot. With `--tokenizer-worker=` (pipelined modes only), +blobExec starts `parallelism` workers and talks to each over its stdin/stdout. +One request in flight per worker. The output contract is byte-identical to `tokenBySha.pl`: same tokens on stdout, same memo file written at `$BFG_MEMO_DIR/xx/yy/`, same @@ -13,7 +14,7 @@ worker must never change bytes. ## Startup ``` -tokenWorker.pl # no args; reads BFG_MEMO_DIR and BFG_TOKENIZE_CMD from env +tokenWorker.pl # no args; inherits blobExec's environment, see below ``` Worker writes `READY\n` to stdout once, then waits for requests. stderr is for @@ -43,28 +44,33 @@ RES \n `33` = parser crash (`$PARSER_CRASH_EXIT`); `124` = the worker killed the parser on timeout; any other non-zero = tokenizer error, stdout empty. - On a non-zero exit nothing is memoized, exactly as `tokenBySha.pl` does. +- stderr holds the request's diagnostics, including what the persistent + helpers wrote during it. blobExec logs it only when the blob fails. ## Timeout The worker runs the parser chain in its own process group (`setpgrp` in the child) and enforces `timeoutSecs` with `alarm`; on expiry it `kill -TERM` then `-KILL`s the group and answers `RES 124 0 `. blobExec keeps a backstop: -if no `RES` header arrives within `timeoutSecs + grace + slack`, it destroys the -worker, respawns one, and reports the blob as a timeout (`Skip`, never `Replace`). +if no `RES` header arrives within `ChildRunner.maxLifetimeSeconds(timeoutSecs)` +(`timeoutSecs + 10`), it kills the worker and its children, starts a new one, +and reports the blob as a timeout (`Skip`, never `Replace`). ## Lifecycle - EOF on stdin → worker exits 0. - A malformed header → worker writes the reason to stderr and exits 2. blobExec respawns and reports that blob as a Skip. -- Workers hold no cross-request state except the memo dir. Order of requests is - irrelevant. - -## Stages - -- Stage A (this branch): worker does dispatch + sha1 + memo + temp file - in-process and spawns only the language parser (`tokenizeSrcMl.pl`, which in - turn runs ctags and `srcml | srcml2token`). Removes `timeout`, `sh`, - `tokenize.pl`, `tokenBySha.pl` from every blob. -- Stage B (later): fold `tokenizeSrcMl.pl` into the worker as a module so only - ctags, srcml and srcml2token are spawned per blob. +- Each worker keeps one `ctags --_interactive` per language and one + `srcml2token --server` alive between requests (`tokenize/CregitSrcMl.pm`), + and starts them again after a timeout or a crash. Only `srcml` is spawned per + blob. Request order is irrelevant. +- Languages that `tokenize.pl` does not send to `tokenizeSrcMl.pl`, and any + `BFG_TOKENIZE_CMD` the worker does not recognise, still spawn + `BFG_TOKENIZE_CMD` per blob. + +## Environment + +- `BFG_MEMO_DIR`, `BFG_TOKENIZE_CMD`: as for `tokenBySha.pl`. +- `BFG_WORKER_INPROC=0`: spawn `BFG_TOKENIZE_CMD` for each blob instead of + the in-process srcML chain (A/B measurement only). From 08723d174af4af29b62cdac5b60e20f32c822332 Mon Sep 17 00:00:00 2001 From: Ellian Carlos Date: Sun, 4 Oct 2026 14:23:30 +0000 Subject: [PATCH 21/25] refactor(tests): split the token worker driver into checks main ran every check in one function (radon C, 15): byte identity, memo hit, unknown extension, timeout and in-process checks. Each group is now a named function, one helper sends a fixture, and one writes the fake scripts. The worst function is now in_process_checks (B, 8). The checks and their order are the same. --- tests/token_worker_driver.py | 117 +++++++++++++++++------------------ 1 file changed, 56 insertions(+), 61 deletions(-) diff --git a/tests/token_worker_driver.py b/tests/token_worker_driver.py index b26331e0..c083a7c3 100755 --- a/tests/token_worker_driver.py +++ b/tests/token_worker_driver.py @@ -4,6 +4,7 @@ import hashlib import os import pathlib +import shutil import subprocess import tempfile import time @@ -105,32 +106,33 @@ def assert_no_timeout_process(marker): ) -def in_process_checks(args, marker, expected_first): - real_srcml = subprocess.run(["which", "srcml"], stdout=subprocess.PIPE, check=True).stdout.decode().strip() - fake_srcml = args.temp_root / "fake-srcml.sh" - fake_srcml.write_text( - "#!/usr/bin/env bash\n" +def write_script(path, body): + path.write_text("#!/usr/bin/env bash\n" + body, encoding="utf-8") + path.chmod(0o755) + return path + + +def in_process_command(args, marker): + fake_srcml = write_script( + args.temp_root / "fake-srcml.sh", 'src="${@: -3:1}"\n' f'grep -q SLEEP_MARKER "$src" && exec perl -e "sleep 100" {marker}\n' 'grep -q ABORT_MARKER "$src" && kill -ABRT $$\n' - f'exec {real_srcml} "$@"\n', - encoding="utf-8", + f'exec {shutil.which("srcml")} "$@"\n', ) - fake_srcml.chmod(0o755) - real_ctags = subprocess.run(["which", "ctags"], stdout=subprocess.PIPE, check=True).stdout.decode().strip() - fake_ctags = args.temp_root / "fake-ctags.sh" - fake_ctags.write_text( - "#!/usr/bin/env bash\n" + fake_ctags = write_script( + args.temp_root / "fake-ctags.sh", "echo HELPER_STDERR_MARKER >&2\n" - f'exec {real_ctags} "$@"\n', - encoding="utf-8", + f'exec {shutil.which("ctags")} "$@"\n', ) - fake_ctags.chmod(0o755) + options = [o for o in args.tokenize_command.split() if not o.startswith(("--srcml=", "--ctags="))] + return " ".join(options + [f"--srcml={fake_srcml}", f"--ctags={fake_ctags}"]) + + +def in_process_checks(args, marker, fixture, expected): memo = args.temp_root / "inproc-memo" memo.mkdir() - options = [o for o in args.tokenize_command.split() if not o.startswith(("--srcml=", "--ctags="))] - command = " ".join(options + [f"--srcml={fake_srcml}", f"--ctags={fake_ctags}"]) - worker = Worker(args.worker, memo, command) + worker = Worker(args.worker, memo, in_process_command(args, marker)) started = time.monotonic() timed_out = worker.request("timeout.c", "fixtures/timeout.c", b"int SLEEP_MARKER;\n", 1) @@ -149,27 +151,18 @@ def in_process_checks(args, marker, expected_first): if any(memo.rglob("*")): raise AssertionError("failed in-process requests were memoized") - recovered = worker.request(args.fixture[0].name, f"fixtures/{args.fixture[0].name}", - args.fixture[0].read_bytes(), 20) - if recovered != (0, expected_first, b""): + recovered = request_fixture(worker, fixture) + if recovered != (0, expected, b""): raise AssertionError(f"in-process worker did not recover: {recovered!r}") worker.close() print("PASS in-process worker recovers after timeout and crash") -def main(): - parser = argparse.ArgumentParser() - parser.add_argument("--worker", required=True, type=pathlib.Path) - parser.add_argument("--token-by-sha", required=True, type=pathlib.Path) - parser.add_argument("--tokenize-command", required=True) - parser.add_argument("--fixture", action="append", required=True, type=pathlib.Path) - parser.add_argument("--temp-root", required=True, type=pathlib.Path) - args = parser.parse_args() +def request_fixture(worker, fixture): + return worker.request(fixture.name, f"fixtures/{fixture.name}", fixture.read_bytes(), 20) - worker_memo = args.temp_root / "worker-memo" - worker_memo.mkdir() - worker = Worker(args.worker, worker_memo, args.tokenize_command) +def byte_identity_checks(args, worker, worker_memo): expected_outputs = [] for index, fixture in enumerate(args.fixture): reference_memo = args.temp_root / f"reference-memo-{index}" @@ -182,51 +175,34 @@ def main(): f"reference failed for {fixture}: " f"{expected.stderr.decode(errors='replace')}" ) - actual = worker.request( - fixture.name, - f"fixtures/{fixture.name}", - fixture.read_bytes(), - 20, - ) + actual = request_fixture(worker, fixture) if actual[0] != 0 or actual[1] != expected.stdout: raise AssertionError(f"worker output differs for {fixture}") if memo_file(worker_memo, fixture.read_bytes()).read_bytes() != expected.stdout: raise AssertionError(f"worker memo differs for {fixture}") expected_outputs.append(expected.stdout) print(f"PASS byte-identical: {fixture.name}") + return expected_outputs - repeated = worker.request( - args.fixture[0].name, - f"fixtures/{args.fixture[0].name}", - args.fixture[0].read_bytes(), - 20, - ) - if repeated != (0, expected_outputs[0], b""): + +def memo_and_recovery_checks(worker, fixture, expected): + if request_fixture(worker, fixture) != (0, expected, b""): raise AssertionError("memo-hit response changed bytes") print("PASS repeated request uses identical memo bytes") unknown = worker.request("unknown.xyzzy", "fixtures/unknown.xyzzy", b"x", 20) if unknown[0] == 0 or b"unknown file extension" not in unknown[2]: raise AssertionError(f"unknown extension was not rejected: {unknown!r}") - recovered = worker.request( - args.fixture[0].name, - f"fixtures/{args.fixture[0].name}", - args.fixture[0].read_bytes(), - 20, - ) - if recovered[0] != 0 or recovered[1] != expected_outputs[0]: + recovered = request_fixture(worker, fixture) + if recovered[0] != 0 or recovered[1] != expected: raise AssertionError("worker did not recover after unknown extension") print("PASS unknown extension is non-fatal to worker") - worker.close() - timeout_script = args.temp_root / "timeout-parser.sh" - marker = f"cregit-token-worker-timeout-{os.getpid()}" - timeout_script.write_text( - "#!/usr/bin/env bash\n" - f"exec perl -e 'sleep 100' {marker}\n", - encoding="utf-8", + +def timeout_checks(args, marker): + timeout_script = write_script( + args.temp_root / "timeout-parser.sh", f"exec perl -e 'sleep 100' {marker}\n" ) - timeout_script.chmod(0o755) timeout_memo = args.temp_root / "timeout-memo" timeout_memo.mkdir() timeout_worker = Worker(args.worker, timeout_memo, str(timeout_script)) @@ -241,7 +217,26 @@ def main(): timeout_worker.close() print(f"PASS timeout exits 124 in {elapsed:.2f}s with no child left") - in_process_checks(args, marker, expected_outputs[0]) + +def main(): + parser = argparse.ArgumentParser() + parser.add_argument("--worker", required=True, type=pathlib.Path) + parser.add_argument("--token-by-sha", required=True, type=pathlib.Path) + parser.add_argument("--tokenize-command", required=True) + parser.add_argument("--fixture", action="append", required=True, type=pathlib.Path) + parser.add_argument("--temp-root", required=True, type=pathlib.Path) + args = parser.parse_args() + + worker_memo = args.temp_root / "worker-memo" + worker_memo.mkdir() + worker = Worker(args.worker, worker_memo, args.tokenize_command) + expected_outputs = byte_identity_checks(args, worker, worker_memo) + memo_and_recovery_checks(worker, args.fixture[0], expected_outputs[0]) + worker.close() + + marker = f"cregit-token-worker-timeout-{os.getpid()}" + timeout_checks(args, marker) + in_process_checks(args, marker, args.fixture[0], expected_outputs[0]) print("ALL TOKEN WORKER TESTS PASSED") From 653743b0c5047eeea8a426b8a194ad3dfac807de Mon Sep 17 00:00:00 2001 From: Ellian Carlos Date: Sun, 4 Oct 2026 14:23:55 +0000 Subject: [PATCH 22/25] style(tokenWorker): drop comments that restate the code The four section dividers in CregitSrcMl.pm name what the subs below them already say, the comment over in_process_tokenizer describes its guard clauses, and the first header line of test_token_worker.sh repeats the file name. --- tests/test_token_worker.sh | 1 - tokenize/CregitSrcMl.pm | 8 -------- tokenizeByBlobId/tokenWorker.pl | 1 - 3 files changed, 10 deletions(-) diff --git a/tests/test_token_worker.sh b/tests/test_token_worker.sh index 251f88dd..4751b571 100755 --- a/tests/test_token_worker.sh +++ b/tests/test_token_worker.sh @@ -1,5 +1,4 @@ #!/usr/bin/env bash -# Byte-identity and lifecycle tests for tokenizeByBlobId/tokenWorker.pl. # Run inside `devenv shell` (needs perl, python3, srcml, ctags on PATH). set -euo pipefail diff --git a/tokenize/CregitSrcMl.pm b/tokenize/CregitSrcMl.pm index 53ce499b..88d8a560 100644 --- a/tokenize/CregitSrcMl.pm +++ b/tokenize/CregitSrcMl.pm @@ -136,8 +136,6 @@ sub parser_crash { return ($PARSER_CRASH_EXIT, "", $stderr); } -# --- ctags ------------------------------------------------------------------- - sub ctags_for { my ($self, $language) = @_; my $proc = $self->{ctagsProc}{$language}; @@ -221,8 +219,6 @@ sub json_field { return json_str($1); } -# --- srcml ------------------------------------------------------------------- - sub run_srcml { my ($self, $language, $inputName, $workDir, $xmlPath) = @_; pipe(my $errRead, my $errWrite) or die "unable to create stderr pipe: $!\n"; @@ -260,8 +256,6 @@ sub run_srcml { return ($status, $stderr); } -# --- srcml2token ------------------------------------------------------------- - sub s2t { my ($self) = @_; return $self->{s2t} if $self->{s2t}; @@ -298,8 +292,6 @@ sub parse_xml { return ($status, \@tokens, $stderr); } -# --- process plumbing -------------------------------------------------------- - sub spawn_pipe { my ($command) = @_; pipe(my $childIn, my $parentOut) or die "unable to create pipe: $!\n"; diff --git a/tokenizeByBlobId/tokenWorker.pl b/tokenizeByBlobId/tokenWorker.pl index 4bfaca8f..0604c97a 100755 --- a/tokenizeByBlobId/tokenWorker.pl +++ b/tokenizeByBlobId/tokenWorker.pl @@ -67,7 +67,6 @@ $inProcess->shutdown() if $inProcess; exit 0; -# In-process srcML only for the stock dispatcher/parser with options the worker honors. sub in_process_tokenizer { my ($program, @options) = @_; return undef if ($ENV{BFG_WORKER_INPROC} // "1") eq "0"; From d34faca6531f266a58113b0fd07f2d8595b552e0 Mon Sep 17 00:00:00 2001 From: Ellian Carlos Date: Sun, 4 Oct 2026 14:26:36 +0000 Subject: [PATCH 23/25] refactor(tokenWorker): split the long worker subs process_request (66 lines) resolved the language, read the memo, wrote the temp input, chose a tokenizer and wrote the memo. run_command (88 lines) set up the child, read both pipes and mapped the status. Each step is now a named sub: tokenize_input, read_memo, write_input, write_memo, exec_in_group and read_streams. In CregitSrcMl.pm, json_str's chain of eight ternaries is a lookup table, and the tag parsing in read_declarations is declaration_from_tag. Outputs are unchanged: the worker tests pass, also with BFG_WORKER_INPROC=0 for the byte-identity checks, and json_str gives the same bytes as before for every escape. --- tokenize/CregitSrcMl.pm | 49 +++++---- tokenizeByBlobId/tokenWorker.pl | 169 ++++++++++++++++---------------- 2 files changed, 114 insertions(+), 104 deletions(-) diff --git a/tokenize/CregitSrcMl.pm b/tokenize/CregitSrcMl.pm index 88d8a560..16b70064 100644 --- a/tokenize/CregitSrcMl.pm +++ b/tokenize/CregitSrcMl.pm @@ -178,38 +178,45 @@ sub read_declarations { $self->drop_ctags($language); die "ctags rejected the request: $line"; } - next unless $line =~ /"_type": "tag"/; - - my ($n) = $line =~ /"line": ([0-9]+)/; - my $name = json_field($line, "name"); - my $kind = json_field($line, "kind"); - my $sig = json_field($line, "signature"); - next unless defined $n and defined $name and defined $kind; - $sig = "-" unless defined $sig; - for ($name, $kind, $sig) { s/\\/\\\\/g } - - my $xref = "$n $name @@@ $kind @@@ $sig"; - die "unable to parse output [$xref]" unless $xref =~ /^([0-9]+) (.+) @@@ (.*) @@@ (.*)$/; - my %decl = (line => $1, name => $2, type => $3, sig => $4); - $decl{name} .= " " . $decl{sig} if $decl{sig} ne "-"; - $declarations{$decl{line}}{$decl{name}} = \%decl; - push @{ $listDeclarations{$decl{line}} }, $decl{name}; + my $decl = declaration_from_tag($line) or next; + $declarations{$decl->{line}}{$decl->{name}} = $decl; + push @{ $listDeclarations{$decl->{line}} }, $decl->{name}; } return (\%declarations, \%listDeclarations); } +sub declaration_from_tag { + my ($line) = @_; + return undef unless $line =~ /"_type": "tag"/; + + my ($n) = $line =~ /"line": ([0-9]+)/; + my $name = json_field($line, "name"); + my $kind = json_field($line, "kind"); + my $sig = json_field($line, "signature"); + return undef unless defined $n and defined $name and defined $kind; + $sig = "-" unless defined $sig; + for ($name, $kind, $sig) { s/\\/\\\\/g } + + my $xref = "$n $name @@@ $kind @@@ $sig"; + die "unable to parse output [$xref]" unless $xref =~ /^([0-9]+) (.+) @@@ (.*) @@@ (.*)$/; + my %decl = (line => $1, name => $2, type => $3, sig => $4); + $decl{name} .= " " . $decl{sig} if $decl{sig} ne "-"; + return \%decl; +} + sub drop_ctags { my ($self, $language) = @_; $self->retire(delete $self->{ctagsProc}{$language}); } +my %JSON_ESCAPE = ( + '"' => '"', '\\' => '\\', '/' => '/', + b => "\b", f => "\f", n => "\n", r => "\r", t => "\t", +); + sub json_str { my ($s) = @_; - $s =~ s/\\(["\\\/bfnrt]|u[0-9a-fA-F]{4})/ - my $e = $1; - $e eq '"' ? '"' : $e eq '\\' ? '\\' : $e eq '\/' ? '\/' : - $e eq 'b' ? "\b" : $e eq 'f' ? "\f" : $e eq 'n' ? "\n" : - $e eq 'r' ? "\r" : $e eq 't' ? "\t" : chr(hex(substr($e, 1)))/ge; + $s =~ s{\\(["\\/bfnrt]|u[0-9a-fA-F]{4})}{$JSON_ESCAPE{$1} // chr(hex(substr($1, 1)))}ge; return $s; } diff --git a/tokenizeByBlobId/tokenWorker.pl b/tokenizeByBlobId/tokenWorker.pl index 0604c97a..22aa4dfc 100755 --- a/tokenizeByBlobId/tokenWorker.pl +++ b/tokenizeByBlobId/tokenWorker.pl @@ -118,54 +118,57 @@ sub protocol_error { sub process_request { my ($origSha, $filename, $fullPath, $contents, $timeoutSecs) = @_; - my ($fileExt, $language); - if ($filename =~ /\.([^.]+)\z/) { - $fileExt = lc($1); - $language = CregitLanguages::language_for_ext($1); - } - if (not defined($language)) { - my $shownExt = defined($fileExt) ? $fileExt : ""; - return (255, "", "unknown file extension [$shownExt]\n"); - } + my ($ext) = $filename =~ /\.([^.]+)\z/; + my $language = defined($ext) ? CregitLanguages::language_for_ext($ext) : undef; + return (255, "", "unknown file extension [" . lc($ext // "") . "]\n") if not defined($language); my $sha1 = sha1_hex($contents); - my $dir = "$memoDir/" . substr($sha1, 0, 2) . "/" . substr($sha1, 2, 2); - my $memoFile = "$dir/$sha1"; - - if (-f $memoFile) { - open(my $memo, '<:raw', $memoFile) - or die "unable to open memoized file [$memoFile]: $!\n"; - local $/; - my $output = <$memo>; - close($memo) or die "unable to close memoized file [$memoFile]: $!\n"; - return (0, $output, ""); - } + my $memoFile = "$memoDir/" . substr($sha1, 0, 2) . "/" . substr($sha1, 2, 2) . "/$sha1"; + return (0, read_memo($memoFile), "") if -f $memoFile; my $tempDir = File::Temp->newdir( "tokdir-XXXXX", DIR => $buildDir, CLEANUP => 1, ); - my $inputName = "input.$fileExt"; - my $inputFile = "$tempDir/$inputName"; + my $inputName = "input." . lc($ext); + write_input("$tempDir/$inputName", $contents); + + my ($exitCode, $output, $error) = tokenize_input($language, $inputName, "$tempDir", $timeoutSecs); + return ($exitCode, "", $error) if $exitCode != 0; + + write_memo($memoFile, $output); + return (0, $output, $error); +} + +sub tokenize_input { + my ($language, $inputName, $workDir, $timeoutSecs) = @_; + return $inProcess->tokenize($language, $inputName, $workDir, $timeoutSecs) + if $inProcess and srcml_language($language); + return run_command($workDir, $timeoutSecs, @tokenizeCommand, "--language=$language", $inputName); +} + +sub read_memo { + my ($memoFile) = @_; + open(my $memo, '<:raw', $memoFile) + or die "unable to open memoized file [$memoFile]: $!\n"; + local $/; + my $output = <$memo>; + close($memo) or die "unable to close memoized file [$memoFile]: $!\n"; + return $output; +} + +sub write_input { + my ($inputFile, $contents) = @_; open(my $input, '>:raw', $inputFile) or die "unable to write temp input [$inputFile]: $!\n"; print {$input} $contents; close($input) or die "unable to close temp input [$inputFile]: $!\n"; +} - my ($exitCode, $output, $error); - if ($inProcess and srcml_language($language)) { - ($exitCode, $output, $error) = $inProcess->tokenize( - $language, $inputName, "$tempDir", $timeoutSecs - ); - } else { - my @command = (@tokenizeCommand, "--language=$language", $inputName); - ($exitCode, $output, $error) = run_command( - "$tempDir", $timeoutSecs, @command - ); - } - return ($exitCode, "", $error) if $exitCode != 0; - +sub write_memo { + my ($memoFile, $output) = @_; + my $dir = dirname($memoFile); make_path($dir) if not -d $dir; my ($memoOut, $tempOutput) = tempfile( "tmpfile-out-XXXXX", @@ -177,8 +180,6 @@ sub process_request { close($memoOut) or die "unable to close tokenizer output [$tempOutput]: $!\n"; move($tempOutput, $memoFile) or die "unable to move tokenizer output to [$memoFile]: $!\n"; - - return (0, $output, $error); } sub run_command { @@ -192,29 +193,11 @@ sub run_command { if ($pid == 0) { close($stdoutRead); close($stderrRead); - open(STDOUT, '>&', $stdoutWrite) or _exit(255); - open(STDERR, '>&', $stderrWrite) or _exit(255); - binmode STDOUT; - binmode STDERR; - close($stdoutWrite); - close($stderrWrite); - setpgrp(0, 0) or do { - print STDERR "unable to create tokenizer process group: $!\n"; - _exit(255); - }; - chdir($workingDir) or do { - print STDERR "unable to enter tokenizer directory [$workingDir]: $!\n"; - _exit(255); - }; - { no warnings 'exec'; exec {$command[0]} @command; } - print STDERR "unable to execute tokenizer [$command[0]]: $!\n"; - _exit(255); + exec_in_group($workingDir, $stdoutWrite, $stderrWrite, @command); } close($stdoutWrite); close($stderrWrite); - binmode $stdoutRead; - binmode $stderrRead; my $timedOut = 0; local $SIG{ALRM} = sub { @@ -224,37 +207,11 @@ sub run_command { kill 'KILL', -$pid; }; alarm($timeoutSecs); - - my $stdoutFd = fileno($stdoutRead); - my $stderrFd = fileno($stderrRead); - my $selector = IO::Select->new($stdoutRead, $stderrRead); - my %stream = ( - $stdoutFd => "", - $stderrFd => "", - ); - while ($selector->count()) { - for my $handle ($selector->can_read()) { - my $chunk = ""; - my $read = sysread($handle, $chunk, 65536); - if (not defined($read)) { - next if $! == EINTR; - die "unable to read tokenizer output: $!\n"; - } - if ($read == 0) { - $selector->remove($handle); - close($handle); - next; - } - $stream{fileno($handle)} .= $chunk; - } - } - + my ($stdout, $stderr) = read_streams($stdoutRead, $stderrRead); waitpid($pid, 0); my $status = $?; alarm(0); - my $stdout = $stream{$stdoutFd}; - my $stderr = $stream{$stderrFd}; if ($timedOut) { $stderr .= "tokenize command timed out after [$timeoutSecs] seconds\n"; return (124, "", $stderr); @@ -270,3 +227,49 @@ sub run_command { return ($status >> 8, $stdout, $stderr); } + +sub exec_in_group { + my ($workingDir, $stdoutWrite, $stderrWrite, @command) = @_; + open(STDOUT, '>&', $stdoutWrite) or _exit(255); + open(STDERR, '>&', $stderrWrite) or _exit(255); + binmode STDOUT; + binmode STDERR; + close($stdoutWrite); + close($stderrWrite); + setpgrp(0, 0) or do { + print STDERR "unable to create tokenizer process group: $!\n"; + _exit(255); + }; + chdir($workingDir) or do { + print STDERR "unable to enter tokenizer directory [$workingDir]: $!\n"; + _exit(255); + }; + { no warnings 'exec'; exec {$command[0]} @command; } + print STDERR "unable to execute tokenizer [$command[0]]: $!\n"; + _exit(255); +} + +sub read_streams { + my ($stdoutRead, $stderrRead) = @_; + binmode $stdoutRead; + binmode $stderrRead; + my %stream = (fileno($stdoutRead) => "", fileno($stderrRead) => ""); + my @order = (fileno($stdoutRead), fileno($stderrRead)); + my $selector = IO::Select->new($stdoutRead, $stderrRead); + while ($selector->count()) { + for my $handle ($selector->can_read()) { + my $read = sysread($handle, my $chunk, 65536); + if (not defined($read)) { + next if $! == EINTR; + die "unable to read tokenizer output: $!\n"; + } + if ($read == 0) { + $selector->remove($handle); + close($handle); + next; + } + $stream{fileno($handle)} .= $chunk; + } + } + return @stream{@order}; +} From dd713bcc8b63420cb27945c070a7f114cb82aa2a Mon Sep 17 00:00:00 2001 From: Ellian Carlos Date: Sun, 4 Oct 2026 14:28:29 +0000 Subject: [PATCH 24/25] refactor(blobExec): share the pool's timed read invoke and startWorker each built the same daemon thread, latch and result cell to read from the worker under a deadline. readWithin does it once and answers None on a timeout. invoke then matches three cases instead of an if around a match, startWorker's three failure messages move to startFailure, and the stderr forwarder writes each line through one local function. Messages and timeouts are unchanged. --- .../cregit/blobexec/TokenizerWorkerPool.scala | 114 ++++++++---------- 1 file changed, 50 insertions(+), 64 deletions(-) diff --git a/blobExec/src/main/scala/cregit/blobexec/TokenizerWorkerPool.scala b/blobExec/src/main/scala/cregit/blobexec/TokenizerWorkerPool.scala index aa03e69c..a81025b0 100644 --- a/blobExec/src/main/scala/cregit/blobexec/TokenizerWorkerPool.scala +++ b/blobExec/src/main/scala/cregit/blobexec/TokenizerWorkerPool.scala @@ -15,6 +15,7 @@ final class TokenizerWorkerPool( require(size > 0, "worker pool size must be positive") private val WorkerTimeoutExitCode = 124 + private val ReadySeconds = 30L private final class Response(val exitCode: Int, val stdout: Array[Byte], val stderr: String) @@ -63,40 +64,21 @@ final class TokenizerWorkerPool( val worker = idle.take() worker.takeDiagnostics() - try { - writeRequest(worker, bytes, origSha, filename, fullPath, timeoutSeconds) - } catch { - case failure: IOException => - return failedWorker(worker, failure) - } - - val finished = new CountDownLatch(1) - val response = new AtomicReference[Either[Throwable, Response]]() - val reader = new Thread( - () => { - try response.set(Right(readResponse(worker.stdout))) - catch { case failure: Throwable => response.set(Left(failure)) } - finally finished.countDown() - }, - s"token-worker-response-${worker.index}" - ) - reader.setDaemon(true) - reader.start() + try writeRequest(worker, bytes, origSha, filename, fullPath, timeoutSeconds) + catch { case failure: IOException => return failedWorker(worker, failure) } val backstopSeconds = ChildRunner.maxLifetimeSeconds(timeoutSeconds) - if (!finished.await(backstopSeconds, TimeUnit.SECONDS)) { - replaceWorker(worker) - ChildRunner.Outcome.Killed(s"no response within ${backstopSeconds}s; worker replaced") - } else { - response.get() match { - case Right(result) => - returnWorker(worker) - if (result.exitCode == WorkerTimeoutExitCode) - ChildRunner.Outcome.Killed(s"no exit within ${timeoutSeconds}s") - else ChildRunner.Outcome.Exited(result.exitCode, result.stdout, result.stderr) - case Left(failure) => - failedWorker(worker, failure) - } + readWithin(s"token-worker-response-${worker.index}", backstopSeconds)(readResponse(worker.stdout)) match { + case None => + replaceWorker(worker) + ChildRunner.Outcome.Killed(s"no response within ${backstopSeconds}s; worker replaced") + case Some(Left(failure)) => + failedWorker(worker, failure) + case Some(Right(result)) => + returnWorker(worker) + if (result.exitCode == WorkerTimeoutExitCode) + ChildRunner.Outcome.Killed(s"no exit within ${timeoutSeconds}s") + else ChildRunner.Outcome.Exited(result.exitCode, result.stdout, result.stderr) } } @@ -127,48 +109,52 @@ final class TokenizerWorkerPool( ) startStderrForwarder(worker) - val ready = new AtomicReference[Either[Throwable, String]]() - val readyLatch = new CountDownLatch(1) - val readyReader = new Thread( + readWithin(s"token-worker-ready-$index", ReadySeconds)(readHeader(worker.stdout)) match { + case Some(Right("READY")) => worker + case notReady => + process.destroyForcibly() + throw startFailure(index, notReady) + } + } + + private def startFailure(index: Int, ready: Option[Either[Throwable, String]]): IllegalStateException = + ready match { + case None => + new IllegalStateException(s"tokenizer worker $index did not print READY within $ReadySeconds seconds") + case Some(Right(line)) => + new IllegalStateException(s"tokenizer worker $index printed [$line] instead of READY") + case Some(Left(failure)) => + new IllegalStateException(s"tokenizer worker $index failed before READY", failure) + } + + private def readWithin[A](threadName: String, seconds: Long)(read: => A): Option[Either[Throwable, A]] = { + val result = new AtomicReference[Either[Throwable, A]]() + val done = new CountDownLatch(1) + val reader = new Thread( () => { - try ready.set(Right(readHeader(worker.stdout))) - catch { case failure: Throwable => ready.set(Left(failure)) } - finally readyLatch.countDown() + try result.set(Right(read)) + catch { case failure: Throwable => result.set(Left(failure)) } + finally done.countDown() }, - s"token-worker-ready-$index" + threadName ) - readyReader.setDaemon(true) - readyReader.start() - - if (!readyLatch.await(30, TimeUnit.SECONDS)) { - process.destroyForcibly() - throw new IllegalStateException(s"tokenizer worker $index did not print READY within 30 seconds") - } - ready.get() match { - case Right("READY") => worker - case Right(line) => - process.destroyForcibly() - throw new IllegalStateException(s"tokenizer worker $index printed [$line] instead of READY") - case Left(failure) => - process.destroyForcibly() - throw new IllegalStateException(s"tokenizer worker $index failed before READY", failure) - } + reader.setDaemon(true) + reader.start() + if (done.await(seconds, TimeUnit.SECONDS)) Some(result.get()) else None } private def startStderrForwarder(worker: Worker): Unit = { + def forward(line: String): Unit = { + worker.appendDiagnostic(line) + System.err.println(s"tokenWorker[${worker.index}]: $line") + } val thread = new Thread( () => { val reader = new BufferedReader(new InputStreamReader(worker.process.getErrorStream, UTF_8)) - try { - Iterator.continually(reader.readLine()).takeWhile(_ != null).foreach { line => - worker.appendDiagnostic(line) - System.err.println(s"tokenWorker[${worker.index}]: $line") - } - } catch { + try Iterator.continually(reader.readLine()).takeWhile(_ != null).foreach(forward) + catch { case failure: IOException if worker.process.isAlive => - val line = s"stderr forwarding failed: ${failure.getMessage}" - worker.appendDiagnostic(line) - System.err.println(s"tokenWorker[${worker.index}]: $line") + forward(s"stderr forwarding failed: ${failure.getMessage}") case _: IOException => () } finally try reader.close() catch { case _: IOException => () } }, From b8d031d7c4c35f2c7201e20d172888ac5a78ad93 Mon Sep 17 00:00:00 2001 From: Ellian Carlos Date: Sun, 4 Oct 2026 14:28:56 +0000 Subject: [PATCH 25/25] refactor(tokenize): take the DECL lines out of merge merge nested the loop that writes a line's ctags declarations inside its token loop. declaration_lines writes them, and merge calls it when the token line changes. The byte-identity checks of the worker test cover the output. --- tokenize/CregitSrcMl.pm | 23 ++++++++++++++--------- 1 file changed, 14 insertions(+), 9 deletions(-) diff --git a/tokenize/CregitSrcMl.pm b/tokenize/CregitSrcMl.pm index 16b70064..bddb8b15 100644 --- a/tokenize/CregitSrcMl.pm +++ b/tokenize/CregitSrcMl.pm @@ -108,15 +108,8 @@ sub merge { unless $tokenLine =~ /^([0-9]+|-):([0-9]+|-)\s+(.+)$/; my ($line, $col, $token) = ($1, $2, $3); my $lineNum = $line eq '-' ? 0 : $line; - if ($lineNum != $lastLine) { - my $names = $listDeclarations->{$line}; - for my $name (@{ $names || [] }) { - my $decl = $declarations->{$line}{$name}; - die "Illegal value in get declaration [$line][$name]" unless defined $decl; - $output .= "$line:-|" if $position; - $output .= "DECL|$decl->{type}|$decl->{name}\n"; - } - } + $output .= declaration_lines($line, $declarations, $listDeclarations, $position) + if $lineNum != $lastLine; $lastLine = $lineNum; $output .= "$line:$col|" if $position; $output .= "$token\n"; @@ -128,6 +121,18 @@ sub merge { return $output; } +sub declaration_lines { + my ($line, $declarations, $listDeclarations, $position) = @_; + my $output = ""; + for my $name (@{ $listDeclarations->{$line} || [] }) { + my $decl = $declarations->{$line}{$name}; + die "Illegal value in get declaration [$line][$name]" unless defined $decl; + $output .= "$line:-|" if $position; + $output .= "DECL|$decl->{type}|$decl->{name}\n"; + } + return $output; +} + sub parser_crash { my ($filename, $stderr, $why) = @_; $stderr .= "cregit: tokenization of [$filename] FAILED: $why\n";