From d8bc3408151456fde70c639d06b717dda91d4198 Mon Sep 17 00:00:00 2001 From: Ellian Carlos Date: Sun, 4 Oct 2026 20:45:46 +0000 Subject: [PATCH 1/4] perf(blobexec): add opt-in tokenizer worker pool Step 2 starts tokenBySha.pl once for each blob. Each start loads Perl, a shell, tokenize.pl and the parser again. On a large history this start-up cost is most of the CPU time of step 2. --tokenizer-worker= (blobExec) and --tokenizer-worker (runner) start one tokenizeByBlobId/tokenWorker.pl per processor for the run. A worker reads requests on stdin (WORKER_PROTOCOL.md) and does what tokenBySha.pl does: the same extension table, the same memo and the same exit statuses. It still starts BFG_TOKENIZE_CMD once per blob. The default path does not change. TokenizerWorkerPool returns the tuple of BlobExec.invoke. A worker answer of 124, or no answer within the budget plus 5 s, is TimeoutExitCode, so a timeout and a parser crash take the same paths as before: the blob is excluded with reason=timeout or parser-crash, and is tried once per run. A wedged worker is killed with its children and replaced. Only --pipeline and --pipeline-trees use the pool, so the flag is refused in the other modes. Tests: TokenizerWorkerPoolSpec; tests/test_token_worker.sh compares the worker with tokenBySha.pl on C, C++, Java, Rust, an empty file, an upper-case extension and a srcML crash, and checks a timeout. CI runs it. --- .github/workflows/ci.yml | 3 + .../main/scala/cregit/blobexec/BlobExec.scala | 8 +- .../src/main/scala/cregit/blobexec/Main.scala | 29 +++- .../cregit/blobexec/TokenizerWorkerPool.scala | 142 ++++++++++++++++++ .../main/scala/cregit/blobexec/Walker.scala | 4 +- .../blobexec/TokenizerWorkerPoolSpec.scala | 136 +++++++++++++++++ run_pipeline_process.sh | 14 +- test_retokenize_passthrough.sh | 10 ++ tests/fixtures/Tiny.java | 4 + tests/fixtures/empty.c | 0 tests/fixtures/tiny.c | 1 + tests/fixtures/tiny.cpp | 2 + tests/fixtures/tiny.rs | 3 + tests/fixtures/upper.H | 6 + tests/test_token_worker.sh | 19 +++ tests/token_worker_driver.py | 138 +++++++++++++++++ tokenizeByBlobId/WORKER_PROTOCOL.md | 35 +++++ tokenizeByBlobId/tokenWorker.pl | 138 +++++++++++++++++ 18 files changed, 685 insertions(+), 7 deletions(-) create mode 100644 blobExec/src/main/scala/cregit/blobexec/TokenizerWorkerPool.scala create mode 100644 blobExec/src/test/scala/cregit/blobexec/TokenizerWorkerPoolSpec.scala create mode 100644 tests/fixtures/Tiny.java create mode 100644 tests/fixtures/empty.c create mode 100644 tests/fixtures/tiny.c create mode 100644 tests/fixtures/tiny.cpp create mode 100644 tests/fixtures/tiny.rs create mode 100644 tests/fixtures/upper.H 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/.github/workflows/ci.yml b/.github/workflows/ci.yml index 79765e82c..7d73cc84c 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -74,6 +74,9 @@ jobs: - name: Build & test Rust tokenizer (rust_tokenizer) run: devenv shell -- bash -c 'cd tokenize/rustTokenizer && make && make test' + - name: "Tokenizer worker: same output as tokenBySha.pl" + run: devenv shell -- bash tests/test_token_worker.sh + - name: "Pipeline runner: artifact staleness guard" run: devenv shell -- bash test_ensure_artifacts.sh diff --git a/blobExec/src/main/scala/cregit/blobexec/BlobExec.scala b/blobExec/src/main/scala/cregit/blobexec/BlobExec.scala index 59599dfcc..632db0996 100644 --- a/blobExec/src/main/scala/cregit/blobexec/BlobExec.scala +++ b/blobExec/src/main/scala/cregit/blobexec/BlobExec.scala @@ -115,9 +115,13 @@ object BlobExec { inserter: ObjectInserter, timeoutSeconds: Int = DefaultTimeoutSeconds, onTimeout: () => Unit = () => (), - onParserCrash: () => Unit = () => () + onParserCrash: () => Unit = () => (), + workerPool: Option[TokenizerWorkerPool] = None ): Outcome = { - val (exitCode, stdout, stderr) = invoke(bytes, origSha, filename, fullPath, command, timeoutSeconds) + val (exitCode, stdout, stderr) = workerPool match { + case Some(pool) => pool.invoke(bytes, filename, timeoutSeconds) + case None => invoke(bytes, origSha, filename, fullPath, command, timeoutSeconds) + } if (exitCode == TimeoutExitCode) { // A timed-out child skips exactly one blob. Deliberately *not* routed diff --git a/blobExec/src/main/scala/cregit/blobexec/Main.scala b/blobExec/src/main/scala/cregit/blobexec/Main.scala index db6019ee7..5c486b60c 100644 --- a/blobExec/src/main/scala/cregit/blobexec/Main.scala +++ b/blobExec/src/main/scala/cregit/blobexec/Main.scala @@ -92,7 +92,7 @@ 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=] [--mask-widened] [--tokenizer-identity==,...] [--retokenize=,...] [--memo-dir=] [--blob-timeout=] [--stall-timeout=] + raw"""Usage: blobExec [--abort-on-error] [--pipeline | --pipeline-trees | --shard=K/N] [--warm=] [--mask-widened] [--tokenizer-identity==,...] [--retokenize=,...] [--memo-dir=] [--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 @@ -188,6 +188,11 @@ object Main { | --memo-dir= the memo directory ($$BFG_MEMO_DIR) whose entries | --retokenize must purge. Only read with --retokenize; | passing it alone is an error rather than a no-op. + | --tokenizer-worker= + | send each blob to one of a pool of persistent + | tokenizer processes (tokenizeByBlobId/tokenWorker.pl) + | instead of running once per blob. Its + | output is the same. Needs --pipeline or --pipeline-trees. | --blob-timeout= | wall-clock budget for one invocation | (default ${BlobExec.DefaultTimeoutSeconds}). A child that exceeds it is @@ -265,6 +270,7 @@ object Main { var tokenizerIdentity = TokenizerIdentity.empty var retokenizeExtensions: Set[String] = Set.empty var memoDir: Option[java.nio.file.Path] = None + var tokenizerWorker: Option[java.nio.file.Path] = None flags.foreach { case "--abort-on-error" => abortOnError = true case "--pipeline" => pipeline = true @@ -316,6 +322,8 @@ object Main { sys.exit(1) } warmPath = Some(p) + case t if t.startsWith("--tokenizer-worker=") => + tokenizerWorker = Some(Paths.get(t.stripPrefix("--tokenizer-worker="))) case t if t.startsWith("--blob-timeout=") => val spec = t.stripPrefix("--blob-timeout=") parsePositiveSeconds(spec) match { @@ -426,6 +434,17 @@ object Main { sys.exit(1) } + tokenizerWorker.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 (!(pipeline || pipelineTrees)) { + System.err.println("Error: --tokenizer-worker needs --pipeline or --pipeline-trees") + sys.exit(1) + } + } + if (positional.length != 5) { System.err.println(Usage) sys.exit(1) @@ -476,7 +495,8 @@ object Main { s"maskWidened=$maskWidened denylistEntries=${denylist.size} " + s"tokenizerIdentity=${if (tokenizerIdentity.isEmpty) "none" else tokenizerIdentity.render} " + s"retokenize=${if (retokenizeExtensions.isEmpty) "none" else retokenizeExtensions.toVector.sorted.mkString(",")} " + - s"memoDir=${memoDir.map(_.toString).getOrElse("none")}" + s"memoDir=${memoDir.map(_.toString).getOrElse("none")} " + + s"tokenizerWorker=${tokenizerWorker.map(_.toString).getOrElse("none")}" ) val src: FileRepository = openSrc(srcPath) @@ -565,15 +585,18 @@ object Main { val stats = try { val parallelism = math.max(1, Runtime.getRuntime.availableProcessors) + val workerPool = tokenizerWorker.map(path => new TokenizerWorkerPool(path.toString, parallelism)) 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 ) - walker.run() + try walker.run() + finally workerPool.foreach(_.close()) } finally { mapping.close() dst.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 000000000..47e178c86 --- /dev/null +++ b/blobExec/src/main/scala/cregit/blobexec/TokenizerWorkerPool.scala @@ -0,0 +1,142 @@ +package cregit.blobexec + +import java.io.{BufferedOutputStream, DataInputStream, EOFException, IOException} +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._ + +/** Persistent tokenizer processes that speak tokenizeByBlobId/WORKER_PROTOCOL.md, + * one request in flight per worker. */ +final class TokenizerWorkerPool(workerPath: String, size: Int) extends AutoCloseable { + import TokenizerWorkerPool._ + + private final class Worker(val index: Int, val process: Process) { + val stdin = new BufferedOutputStream(process.getOutputStream) + val stdout = new DataInputStream(process.getInputStream) + } + + private val idle = new ArrayBlockingQueue[Worker](size) + private val workers = ConcurrentHashMap.newKeySet[Worker]() + private val closed = new AtomicBoolean(false) + + try (0 until size).foreach(addWorker) + catch { case failure: Throwable => close(); throw failure } + + /** Same result as [[BlobExec.invoke]]: (exit, stdout, stderr), and + * [[BlobExec.TimeoutExitCode]] for a blob that ran out of time. */ + def invoke( + bytes: Array[Byte], + filename: String, + timeoutSeconds: Int + ): (Int, Array[Byte], String) = { + val worker = idle.take() + val backstop = timeoutSeconds + BackstopSlackSeconds + within(backstop)(request(worker, bytes, filename, timeoutSeconds)) match { + case None => + System.err.println( + s"blobExec: tokenizer worker ${worker.index} gave no response within ${backstop}s; replacing it") + replace(worker) + Timeout + case Some(Left(failure)) => + replace(worker) + (1, Array.emptyByteArray, s"tokenizer worker ${worker.index} failed: $failure\n") + case Some(Right(response)) => + if (worker.process.isAlive) idle.put(worker) else replace(worker) + if (response._1 == WorkerTimeoutExitCode) Timeout else response + } + } + + override def close(): Unit = + if (closed.compareAndSet(false, true)) { + val all = workers.asScala.toVector + all.foreach(w => try w.stdin.close() catch { case _: IOException => () }) + val deadline = System.nanoTime() + TimeUnit.SECONDS.toNanos(CloseSeconds) + all.foreach { w => + if (!w.process.waitFor(math.max(0L, deadline - System.nanoTime()), TimeUnit.NANOSECONDS)) kill(w.process) + } + } + + private def addWorker(index: Int): Unit = { + val process = new ProcessBuilder(workerPath).redirectError(ProcessBuilder.Redirect.INHERIT).start() + val worker = new Worker(index, process) + within(ReadySeconds)(readLine(worker.stdout)) match { + case Some(Right("READY")) => + workers.add(worker) + idle.put(worker) + case notReady => + kill(process) + throw new IllegalStateException(s"tokenizer worker [$workerPath] did not start: $notReady") + } + } + + private def replace(worker: Worker): Unit = { + workers.remove(worker) + kill(worker.process) + if (!closed.get()) addWorker(worker.index) + } + + private def request(worker: Worker, bytes: Array[Byte], filename: String, timeoutSeconds: Int) = { + val name = filename.getBytes(UTF_8) + worker.stdin.write(s"REQ ${bytes.length} ${name.length} $timeoutSeconds\n".getBytes(UTF_8)) + worker.stdin.write(name) + worker.stdin.write(bytes) + worker.stdin.flush() + readLine(worker.stdout).split(' ') match { + case Array("RES", exit, outLength, errLength) => + val out = readBytes(worker.stdout, outLength.toInt) + (exit.toInt, out, new String(readBytes(worker.stdout, errLength.toInt), UTF_8)) + case header => throw new IOException(s"malformed response header [${header.mkString(" ")}]") + } + } +} + +object TokenizerWorkerPool { + /** What a worker answers when it killed the tokenizer for running out of time. */ + val WorkerTimeoutExitCode = 124 + private val Timeout = (BlobExec.TimeoutExitCode, Array.emptyByteArray, "") + /** The worker kills its own tokenizer at the budget. This only catches a wedged worker. */ + private val BackstopSlackSeconds = 5L + private val ReadySeconds = 30L + private val CloseSeconds = 2L + private val MaxHeaderBytes = 8192 + + /** Kill the children first: once the worker is gone they are no longer its descendants. */ + private def kill(process: Process): Unit = { + process.descendants().forEach(child => { child.destroyForcibly(); () }) + process.destroyForcibly() + () + } + + /** Runs `read` on a daemon thread, so that a worker which never answers cannot block the caller. */ + private def within[A](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 result.set(Right(read)) + catch { case failure: Throwable => result.set(Left(failure)) } + finally done.countDown() + }, "tokenizer-worker-read") + reader.setDaemon(true) + reader.start() + if (done.await(seconds, TimeUnit.SECONDS)) Some(result.get()) else None + } + + private def readLine(in: DataInputStream): String = { + val line = new java.io.ByteArrayOutputStream + var next = in.read() + while (next != '\n') { + if (next == -1) throw new EOFException("tokenizer worker closed its stdout") + if (line.size() >= MaxHeaderBytes) throw new IOException(s"header longer than $MaxHeaderBytes bytes") + line.write(next) + next = in.read() + } + line.toString(UTF_8) + } + + private def readBytes(in: DataInputStream, length: Int): Array[Byte] = { + val bytes = new Array[Byte](length) + in.readFully(bytes) + bytes + } +} diff --git a/blobExec/src/main/scala/cregit/blobexec/Walker.scala b/blobExec/src/main/scala/cregit/blobexec/Walker.scala index 095f11b8b..2a730cded 100644 --- a/blobExec/src/main/scala/cregit/blobexec/Walker.scala +++ b/blobExec/src/main/scala/cregit/blobexec/Walker.scala @@ -60,6 +60,7 @@ final class Walker( destinationMayContainObjects: Boolean = true, blobTimeoutSeconds: Int = BlobExec.DefaultTimeoutSeconds, stallTimeoutSeconds: Int = Walker.DefaultStallTimeoutSeconds, + workerPool: Option[TokenizerWorkerPool] = None, // The shipped list by default, so a caller cannot forget it and hand a known // non-terminating blob to srcml. A parameter only so a test can supply its // own fixture; nothing at run time chooses a different list. @@ -803,7 +804,8 @@ final class Walker( inserter = workerInserter, timeoutSeconds = blobTimeoutSeconds, onTimeout = () => { blobsTimedOut.increment(); failure.set("timeout") }, - onParserCrash = () => { blobsParserCrashed.increment(); failure.set("parser-crash") } + onParserCrash = () => { blobsParserCrashed.increment(); failure.set("parser-crash") }, + workerPool = workerPool ) val res = outcome match { case BlobExec.Outcome.Skip if failure.get() != null => 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 000000000..4921b3fbb --- /dev/null +++ b/blobExec/src/test/scala/cregit/blobexec/TokenizerWorkerPoolSpec.scala @@ -0,0 +1,136 @@ +package cregit.blobexec + +import org.eclipse.jgit.internal.storage.dfs.{DfsRepositoryDescription, InMemoryRepository} +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, Paths} +import scala.concurrent.duration._ +import scala.concurrent.{Await, ExecutionContext, Future} +import scala.jdk.CollectionConverters._ + +class TokenizerWorkerPoolSpec extends AnyFunSuite with Matchers with BeforeAndAfterAll { + + private val tmpDir = Files.createTempDirectory("tokenizer-worker-pool-") + private val worker = tmpDir.resolve("fake-worker.pl") + private val grandchildPidFile = Paths.get(s"$worker.grandchild") + + override def beforeAll(): Unit = { + Files.writeString( + worker, + """|#!/usr/bin/env perl + |use strict; + |use warnings; + |$| = 1; + |sub reply { my ($exit, $out) = @_; print "RES $exit ", length($out), " 0\n", $out } + |print "READY\n"; + |while (my $header = ) { + | my (undef, $bodyLength, $nameLength, $timeout) = split ' ', $header; + | read(STDIN, my $name, $nameLength); + | read(STDIN, my $body, $bodyLength) if $bodyLength; + | if ($name eq 'hang.c') { + | my $child = fork() // exit 3; + | exec 'sleep', '300' if $child == 0; + | open(my $pids, '>', "$0.grandchild") or exit 3; + | print {$pids} $child; + | close($pids); + | sleep 300; + | } + | exit 7 if $name eq 'die.c'; + | sleep 2 if $name eq 'slow.c'; + | if ($name eq 'crash.c') { reply(33, '') } + | elsif ($name eq 'timeout.c') { reply(124, '') } + | elsif ($name eq 'budget.c') { reply(0, $timeout) } + | else { reply(0, uc $body) } + |} + |""".stripMargin + ) + worker.toFile.setExecutable(true) + } + + override def afterAll(): Unit = + Files.walk(tmpDir).sorted(java.util.Comparator.reverseOrder()).forEach(p => Files.delete(p)) + + private def withPool[A](size: Int)(body: TokenizerWorkerPool => A): A = { + val pool = new TokenizerWorkerPool(worker.toString, size) + try body(pool) finally pool.close() + } + + private def stdout(pool: TokenizerWorkerPool, filename: String, body: String, timeoutSeconds: Int = 2) = { + val (exit, out, _) = pool.invoke(body.getBytes(UTF_8), filename, timeoutSeconds) + exit shouldEqual 0 + new String(out, UTF_8) + } + + test("returns the worker's output and sends each request's own budget") { + withPool(1) { pool => + stdout(pool, "hello.c", "hello") shouldEqual "HELLO" + stdout(pool, "budget.c", "x", timeoutSeconds = 7) shouldEqual "7" + stdout(pool, "budget.c", "x", timeoutSeconds = 21) shouldEqual "21" + } + } + + test("a worker's timeout and parser crash take BlobExec's timeout and crash paths") { + val inserter = new InMemoryRepository(new DfsRepositoryDescription("pool-spec")).newObjectInserter() + withPool(1) { pool => + def run(filename: String): (BlobExec.Outcome, String) = { + var called = "none" + val outcome = BlobExec.run( + "body".getBytes(UTF_8), "a" * 40, filename, s"src/$filename", "tokenBySha.pl", + abortOnError = true, inserter = inserter, + onTimeout = () => called = "timeout", onParserCrash = () => called = "parser-crash", + workerPool = Some(pool)) + (outcome, called) + } + run("timeout.c") shouldEqual ((BlobExec.Outcome.Skip, "timeout")) + run("crash.c") shouldEqual ((BlobExec.Outcome.Skip, "parser-crash")) + run("ok.c")._1 shouldBe a[BlobExec.Outcome.Replace] + } + } + + test("kills a wedged worker with its children, replaces it, and serves the next request") { + withPool(1) { pool => + val started = System.nanoTime() + pool.invoke("x".getBytes(UTF_8), "hang.c", 1)._1 shouldEqual BlobExec.TimeoutExitCode + (System.nanoTime() - started).nanos should be < 15.seconds + eventuallyStopped(Seq(Files.readString(grandchildPidFile).trim.toLong)) shouldBe true + stdout(pool, "next.c", "next") shouldEqual "NEXT" + } + } + + test("a worker that dies in a request fails that blob only") { + withPool(1) { pool => + pool.invoke("x".getBytes(UTF_8), "die.c", 2)._1 should not equal 0 + stdout(pool, "ok.c", "ok") shouldEqual "OK" + } + } + + test("serves pool-sized requests in parallel") { + implicit val ec: ExecutionContext = ExecutionContext.global + withPool(3) { pool => + val started = System.nanoTime() + val requests = (1 to 3).map(n => Future(stdout(pool, "slow.c", s"body$n", timeoutSeconds = 5))) + Await.result(Future.sequence(requests), 10.seconds).toSet shouldEqual Set("BODY1", "BODY2", "BODY3") + (System.nanoTime() - started).nanos should be < 4.seconds + } + } + + test("close stops every worker") { + val pool = new TokenizerWorkerPool(worker.toString, 2) + val pids = ProcessHandle.current().children().iterator().asScala + .filter(_.info().commandLine().orElse("").contains(worker.toString)) + .map(_.pid()).toVector + pids should have size 2 + pool.close() + eventuallyStopped(pids) shouldBe true + } + + private def eventuallyStopped(pids: Seq[Long]): Boolean = { + def alive(pid: Long) = ProcessHandle.of(pid).map[Boolean](_.isAlive).orElse(false) + val deadline = System.nanoTime() + 5.seconds.toNanos + while (System.nanoTime() < deadline && pids.exists(alive)) Thread.sleep(25) + !pids.exists(alive) + } +} diff --git a/run_pipeline_process.sh b/run_pipeline_process.sh index a57f3cff7..a09827d8f 100755 --- a/run_pipeline_process.sh +++ b/run_pipeline_process.sh @@ -223,6 +223,11 @@ Tokenizer: for repos too large to tokenize in one process; delegates to blobExec/shard_build.sh --shards N shard count for --mode sharded (default: 4) + --tokenizer-worker + send each blob to a pool of persistent tokenizer processes + (tokenizeByBlobId/tokenWorker.pl) instead of starting the + tokenizer once per blob. Same output. Needs --mode pipeline or + pipeline-trees (default: off) --jobs N concurrent blame/HTML processes (default: CREGIT_JOBS, otherwise min(4, available CPUs)). Blame is the pipeline's bottleneck: measured on Linux the serial step managed 8 files @@ -349,6 +354,7 @@ RETOKENIZE="" # does hand its environment to this script. BLOB_TIMEOUT="${CREGIT_BLOB_TIMEOUT:-}" STALL_TIMEOUT="${CREGIT_STALL_TIMEOUT:-}" +TOKENIZER_WORKER=0 # Markers meaning "the work in $WORK is incomplete but recoverable, and a # FROM_STEP=1 wipe would throw away days of tokenizing to redo it". Written by @@ -557,6 +563,7 @@ while [ $# -gt 0 ]; do --mask-widened) MASK_WIDENED=1; shift ;; --retokenize) need_val "$@"; RETOKENIZE="$2"; shift 2 ;; --blob-timeout) need_val "$@"; BLOB_TIMEOUT="$2"; shift 2 ;; + --tokenizer-worker) TOKENIZER_WORKER=1; shift ;; --stall-timeout) need_val "$@"; STALL_TIMEOUT="$2"; shift 2 ;; --gc) need_val "$@"; GC_MODE="$2"; shift 2 ;; --memory-limit) need_val "$@"; MEMORY_LIMIT="$2"; shift 2 ;; @@ -804,6 +811,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 @@ -1252,6 +1262,8 @@ else [ "$MODE" = "pipeline-trees" ] && MODE_FLAG="--pipeline-trees" WIDENED_FLAG="" [ "$MASK_WIDENED" = 1 ] && WIDENED_FLAG="--mask-widened" + WORKER_FLAG="" + [ "$TOKENIZER_WORKER" = 1 ] && WORKER_FLAG="--tokenizer-worker=${CREGIT}/tokenizeByBlobId/tokenWorker.pl" # --retokenize carries --memo-dir with it, always, and blobExec refuses one # without the other: the blob map and the memo are two caches of the same # answer, and invalidating either alone invalidates nothing. @@ -1259,7 +1271,7 @@ else if [ -n "$RETOKENIZE" ]; then RETOKENIZE_FLAGS=("--retokenize=$RETOKENIZE" "--memo-dir=$MEMO_DIR") fi - java -jar "$BFG" $MODE_FLAG $WIDENED_FLAG \ + java -jar "$BFG" $MODE_FLAG $WIDENED_FLAG ${WORKER_FLAG:+"$WORKER_FLAG"} \ "--tokenizer-identity=$TOKENIZER_IDENTITY" \ ${RETOKENIZE_FLAGS[@]+"${RETOKENIZE_FLAGS[@]}"} \ ${BLOB_TIMEOUT:+--blob-timeout=$BLOB_TIMEOUT} \ diff --git a/test_retokenize_passthrough.sh b/test_retokenize_passthrough.sh index 3ec4c3703..b604c212e 100755 --- a/test_retokenize_passthrough.sh +++ b/test_retokenize_passthrough.sh @@ -228,6 +228,16 @@ OUT=$(STUB_RC=0 run_step2 "$W" --retokenize rs --mask-widened --mask '\.c$' 2); [ "$RC" -ne 0 ]; check "refused (exit $RC)" $? rm -rf "$W" +# --------------------------------------------------------------------------- +echo "case 10: --tokenizer-worker reaches blobExec, and only in the pipelined modes" +W=$(fixture); ARGV="$W/argv.log" +OUT=$(STUB_RC=0 STUB_ARGV="$ARGV" run_step2 "$W" --tokenizer-worker 2); RC=$? +grep -q -- "--tokenizer-worker=$REPO/tokenizeByBlobId/tokenWorker.pl" "$ARGV" +check "blobExec was given the worker path" $? +OUT=$(STUB_RC=0 run_step2 "$W" --tokenizer-worker --mode serial 2); RC=$? +[ "$RC" -ne 0 ] && grep -q 'needs --mode pipeline' <<<"$OUT"; check "--mode serial is refused (exit $RC)" $? +rm -rf "$W" + # --------------------------------------------------------------------------- echo "" echo "passed: $PASS failed: $FAIL" diff --git a/tests/fixtures/Tiny.java b/tests/fixtures/Tiny.java new file mode 100644 index 000000000..c9b995f9b --- /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 000000000..e69de29bb diff --git a/tests/fixtures/tiny.c b/tests/fixtures/tiny.c new file mode 100644 index 000000000..78f2de106 --- /dev/null +++ b/tests/fixtures/tiny.c @@ -0,0 +1 @@ +int main(void) { return 0; } diff --git a/tests/fixtures/tiny.cpp b/tests/fixtures/tiny.cpp new file mode 100644 index 000000000..88e4ae717 --- /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/fixtures/tiny.rs b/tests/fixtures/tiny.rs new file mode 100644 index 000000000..2954eacc2 --- /dev/null +++ b/tests/fixtures/tiny.rs @@ -0,0 +1,3 @@ +fn add(a: i32, b: i32) -> i32 { + a + b // "x|y" +} diff --git a/tests/fixtures/upper.H b/tests/fixtures/upper.H new file mode 100644 index 000000000..e039d96f3 --- /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 new file mode 100755 index 000000000..f3d794f87 --- /dev/null +++ b/tests/test_token_worker.sh @@ -0,0 +1,19 @@ +#!/usr/bin/env bash +# Run inside `devenv shell`: it needs perl, python3, srcml, ctags and cargo. +set -euo pipefail + +ROOT=$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd) +SRCML2TOKEN="$ROOT/tokenize/srcMLtoken/srcml2token" +[[ -x "$SRCML2TOKEN" ]] || make -C "$ROOT/tokenize/srcMLtoken" +[[ -x "$ROOT/tokenize/rustTokenizer/target/release/rust_tokenizer" ]] || make -C "$ROOT/tokenize/rustTokenizer" + +TEMP_ROOT=$(mktemp -d "${TMPDIR:-/tmp}/cregit-token-worker.XXXXXX") +trap 'rm -rf "$TEMP_ROOT"' EXIT + +FIXTURES=("$ROOT"/tests/fixtures/* "$ROOT/tokenize/t/fixtures/srcml-position-crash.c") +python3 "$ROOT/tests/token_worker_driver.py" \ + --worker "$ROOT/tokenizeByBlobId/tokenWorker.pl" \ + --token-by-sha "$ROOT/tokenizeByBlobId/tokenBySha.pl" \ + --tokenize-command "$ROOT/tokenize/tokenize.pl --srcml2token=$SRCML2TOKEN --srcml=$(command -v srcml) --ctags=$(command -v ctags)" \ + "${FIXTURES[@]/#/--fixture=}" \ + --temp-root "$TEMP_ROOT" diff --git a/tests/token_worker_driver.py b/tests/token_worker_driver.py new file mode 100755 index 000000000..2a270f1e5 --- /dev/null +++ b/tests/token_worker_driver.py @@ -0,0 +1,138 @@ +#!/usr/bin/env python3 +"""Checks tokenizeByBlobId/tokenWorker.pl against tokenizeByBlobId/tokenBySha.pl.""" + +import argparse +import hashlib +import os +import pathlib +import subprocess +import time + +TIMEOUT_EXIT = 124 +PARSER_CRASH_EXIT = 33 + + +class Worker: + def __init__(self, executable, memo_dir, tokenize_command): + env = dict(os.environ, BFG_MEMO_DIR=str(memo_dir), BFG_TOKENIZE_CMD=tokenize_command) + self.process = subprocess.Popen( + [str(executable)], stdin=subprocess.PIPE, stdout=subprocess.PIPE, env=env + ) + ready = self.process.stdout.readline() + if ready != b"READY\n": + raise AssertionError(f"worker did not start: {ready!r}") + + def request(self, filename, contents, timeout_secs=20): + name = os.fsencode(filename) + header = f"REQ {len(contents)} {len(name)} {timeout_secs}\n".encode() + self.process.stdin.write(header + name + contents) + self.process.stdin.flush() + response = self.process.stdout.readline().decode().split() + if len(response) != 4 or response[0] != "RES": + raise AssertionError(f"malformed response header: {response!r}") + exit_code, out_len, err_len = map(int, response[1:]) + return exit_code, self.process.stdout.read(out_len), self.process.stdout.read(err_len) + + def close(self): + self.process.stdin.close() + if self.process.wait(timeout=5) != 0: + raise AssertionError(f"worker exited {self.process.returncode} on EOF") + + +def check(condition, message): + if not condition: + raise AssertionError(message) + print(f"PASS {message}") + + +def token_by_sha(args, memo_dir, fixture): + env = dict( + os.environ, + BFG_FILENAME=fixture.name, + BFG_MEMO_DIR=str(memo_dir), + BFG_TOKENIZE_CMD=args.tokenize_command, + ) + return subprocess.run( + [str(args.token_by_sha)], input=fixture.read_bytes(), capture_output=True, env=env + ) + + +def memo_file(memo_dir, contents): + digest = hashlib.sha1(contents).hexdigest() + return memo_dir / digest[:2] / digest[2:4] / digest + + +def new_dir(args, name): + path = args.temp_root / name + path.mkdir() + return path + + +def write_script(path, body): + path.write_text("#!/usr/bin/env bash\n" + body, encoding="utf-8") + path.chmod(0o755) + return path + + +def no_process_left(marker): + time.sleep(0.2) + return subprocess.run(["pgrep", "-f", marker], capture_output=True).returncode == 1 + + +def timed_request(worker, filename, contents): + started = time.monotonic() + response = worker.request(filename, contents, timeout_secs=1) + return response, time.monotonic() - started + + +def check_same_as_token_by_sha(args): + memo = new_dir(args, "worker-memo") + worker = Worker(args.worker, memo, args.tokenize_command) + for index, fixture in enumerate(args.fixture): + expected = token_by_sha(args, new_dir(args, f"reference-memo-{index}"), fixture) + actual = worker.request(fixture.name, fixture.read_bytes()) + check(actual[0] == expected.returncode, f"{fixture.name}: exit {expected.returncode}") + if expected.returncode == 0: + check(actual[1] == expected.stdout, f"{fixture.name}: same tokens") + check( + memo_file(memo, fixture.read_bytes()).read_bytes() == expected.stdout, + f"{fixture.name}: same memo file", + ) + + first = args.fixture[0] + expected = memo_file(memo, first.read_bytes()).read_bytes() + check(worker.request(first.name, first.read_bytes()) == (0, expected, b""), "a memo hit gives the same bytes") + unknown = worker.request("unknown.xyzzy", b"x") + check(unknown[0] == 255 and b"unknown file extension" in unknown[2], "an unknown extension fails") + check(worker.request(first.name, first.read_bytes())[0] == 0, "the worker serves the next request") + worker.close() + + +def check_timeout(args, marker): + memo = new_dir(args, "timeout-memo") + script = write_script(args.temp_root / "hang.sh", f"exec perl -e 'sleep 100' {marker}\n") + worker = Worker(args.worker, memo, str(script)) + (exit_code, out, _), elapsed = timed_request(worker, "hang.c", b"int x;\n") + check(exit_code == TIMEOUT_EXIT and out == b"" and elapsed < 3, f"a hung tokenizer exits {TIMEOUT_EXIT}") + check(no_process_left(marker), "the timeout kills the tokenizer's process group") + check(not any(memo.rglob("*")), "a timeout memoizes nothing") + worker.close() + + +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() + + marker = f"cregit-token-worker-{os.getpid()}" + check_same_as_token_by_sha(args) + check_timeout(args, marker) + 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 000000000..1b5c4d200 --- /dev/null +++ b/tokenizeByBlobId/WORKER_PROTOCOL.md @@ -0,0 +1,35 @@ +# Tokenizer worker protocol + +`tokenWorker.pl` is a persistent `tokenBySha.pl`. With +`--tokenizer-worker=`, blobExec starts one worker per processor and +sends each blob to an idle worker. A worker handles one request at a time. + +The worker reads `BFG_MEMO_DIR` and `BFG_TOKENIZE_CMD` from its environment, +as `tokenBySha.pl` does. Its tokens, its memo files and its exit statuses are +the same as those of `tokenBySha.pl`. + +## Messages + +At start, the worker writes `READY\n` to stdout. Then, for each request: + +``` +JVM -> worker: REQ \n +worker -> JVM: RES \n +``` + +- `name` is the file name of the blob. Its extension selects the language. +- Lengths are byte counts. The strings are raw bytes. +- `exit` is the status that `tokenBySha.pl` gives: `0` (the tokens are in + `out`), `33` (parser crash) or another status (tokenizer error, `out` is + empty). `124` means that the tokenizer did not finish in `timeoutSeconds`. +- The worker memoizes only a successful tokenization. + +## Failures + +- The worker runs the tokenizer in its own process group. At the timeout it + kills the group and answers `124`. +- If no answer comes in `timeoutSeconds + 5` seconds, blobExec kills the + worker and its children, starts a new worker, and reports a timeout. +- If the worker stops in a request, blobExec reports a tokenizer error for + that blob and starts a new worker. +- At EOF on stdin, the worker exits 0. A malformed request makes it exit 2. diff --git a/tokenizeByBlobId/tokenWorker.pl b/tokenizeByBlobId/tokenWorker.pl new file mode 100755 index 000000000..34c4de2ae --- /dev/null +++ b/tokenizeByBlobId/tokenWorker.pl @@ -0,0 +1,138 @@ +#!/usr/bin/env perl + +# A persistent tokenBySha.pl with the same output and memo. The protocol is in +# WORKER_PROTOCOL.md. + +use strict; +use warnings; +use bytes; +use Digest::SHA qw(sha1_hex); +use File::Basename qw(dirname); +use File::Copy qw(move); +use File::Path qw(make_path); +use File::Temp qw(tempfile); +use FindBin qw($RealBin); +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 $memoDir eq ""; +die "Sha dir [$memoDir] does not exist\n" if not -d $memoDir; +my @tokenizeCommand = split ' ', $ENV{BFG_TOKENIZE_CMD} // ""; +die "Tokenize command not defined. Use BFG_TOKENIZE_CMD\n" if not @tokenizeCommand; +my $buildDir = "$RealBin/build"; +make_path($buildDir); + +print "READY\n"; +while (defined(my $header = )) { + my ($bodyLength, $nameLength, $timeoutSecs) = $header =~ /\AREQ (\d+) (\d+) (\d+)\n\z/ + or protocol_error("malformed request header [$header]"); + my $filename = read_exact($nameLength); + my $contents = read_exact($bodyLength); + my ($exit, $out, $err) = eval { tokenize_blob($filename, $contents, $timeoutSecs) }; + ($exit, $out, $err) = (255, "", $@) if not defined $exit; + print "RES $exit ", length($out), " ", length($err), "\n", $out, $err; +} +exit 0; + +sub read_exact { + my ($length) = @_; + my $value = ""; + while (length($value) < $length) { + my $read = read(STDIN, $value, $length - length($value), length($value)); + protocol_error(defined $read ? "the request ended early" : "cannot read the request: $!") + if not $read; + } + return $value; +} + +sub protocol_error { + print STDERR "tokenWorker.pl: @_\n"; + exit 2; +} + +sub tokenize_blob { + my ($filename, $contents, $timeoutSecs) = @_; + my ($ext) = $filename =~ /\.([^.]+)$/; + $ext = lc($ext // ""); + my $language = $CregitLanguages::EXT_LANG{$ext}; + return (255, "", "unknown file extension [$ext]\n") if not defined $language; + + my $sha1 = sha1_hex($contents); + my $memoFile = "$memoDir/" . substr($sha1, 0, 2) . "/" . substr($sha1, 2, 2) . "/$sha1"; + return (0, read_file($memoFile), "") if -f $memoFile; + + my $workDir = File::Temp->newdir("tokdir-XXXXX", DIR => $buildDir); + write_file("$workDir/input.$ext", $contents); + my ($exit, $out, $err) = run_tokenizer("$workDir", $timeoutSecs, @tokenizeCommand, + "--language=$language", "input.$ext"); + return ($exit, "", $err) if $exit != 0; + write_memo($memoFile, $out); + return (0, $out, $err); +} + +sub run_tokenizer { + my ($workDir, $timeoutSecs, @command) = @_; + my $pid = fork() // die "cannot fork the tokenizer: $!\n"; + exec_in_group($workDir, @command) if $pid == 0; + + my $timedOut = 0; + local $SIG{ALRM} = sub { $timedOut = 1; kill_group($pid) }; + alarm($timeoutSecs); + waitpid($pid, 0); + my $status = $?; + alarm(0); + + my $err = read_file("$workDir/.stderr"); + return (124, "", $err . "tokenize command timed out after [$timeoutSecs] seconds\n") if $timedOut; + return (255, "", $err . "tokenize command was killed by signal " . ($status & 127) . "\n") if $status & 127; + return ($status >> 8, read_file("$workDir/.stdout"), $err); +} + +# The child's own process group lets a timeout kill srcml and everything else +# the tokenizer started. +sub exec_in_group { + my ($workDir, @command) = @_; + setpgrp(0, 0); + chdir($workDir) and open(STDIN, '<', '/dev/null') and open(STDOUT, '>', '.stdout') + and open(STDERR, '>', '.stderr') or _exit(255); + { no warnings 'exec'; exec { $command[0] } @command; } + print STDERR "cannot execute the tokenizer [$command[0]]: $!\n"; + _exit(255); +} + +sub kill_group { + my ($pid) = @_; + kill 'TERM', -$pid; + select(undef, undef, undef, 0.1); + kill 'KILL', -$pid; +} + +sub read_file { + my ($path) = @_; + open(my $in, '<:raw', $path) or die "cannot read [$path]: $!\n"; + local $/; + return <$in> // ""; +} + +sub write_file { + my ($path, $contents) = @_; + open(my $out, '>:raw', $path) or die "cannot write [$path]: $!\n"; + print {$out} $contents; + close($out) or die "cannot close [$path]: $!\n"; +} + +sub write_memo { + my ($memoFile, $output) = @_; + make_path(dirname($memoFile)); + my ($out, $temp) = tempfile("tmpfile-out-XXXXX", DIR => $buildDir); + binmode $out; + print {$out} $output; + close($out) or die "cannot close [$temp]: $!\n"; + move($temp, $memoFile) or die "cannot move the tokenization to [$memoFile]: $!\n"; +} From 23757997ac7f419374f72c75898159e7c16bfdd7 Mon Sep 17 00:00:00 2001 From: Ellian Carlos Date: Sun, 4 Oct 2026 22:35:12 +0000 Subject: [PATCH 2/4] build(devenv): build srcml 1.1.0 from its tag devenv.nix packaged the srcML 1.1.0 release binary. That binary has no libsrcml headers, and the next commit links srcml2token against libsrcml. nix/srcml.nix now builds the v1.1.0 tag from source, with the dependencies of that tag (antlr 2.7.7, TinySHA1, CTPL, CLI11 2.5.0) supplied by Nix, so the sandboxed build needs no network. It installs the srcml binary, libsrcml and its headers. A cold build takes 24 s on 16 cores and 37 s on 4. The tag, not the develop revision 8a3a629d that master builds (#93): on a sweep of 706 C, C++ and Java files, 8a3a629d gives other tokens than the release binary for 13 files that 1.1.0 parses: - 8 files with UTF-8 text: develop reads valid UTF-8 as UTF-8, 1.1.0 reads it as Latin-1, so names and columns change. - 5 files with a top-level macro call: since srcML/srcML#2325 it is an expression, not a macro. The --position crash fix (#2340) comes after #2325, so no develop revision gives the tokens of 1.1.0. The tag gives them on all 706 files. It keeps the two 1.1.0 defects (a crash under --position, a Java hang on srcML/srcML#2361); those blobs stay excluded as parser-crash or timeout. tokenizeSrcMl.t adds a stub srcml that kills itself with SIGSEGV, so the exit-33 path is covered whatever srcml devenv ships. --- README.md | 4 +- devenv.nix | 37 +------------ nix/srcml.nix | 104 +++++++++++++++++++++++++++++++++++++ tokenize/t/tokenizeSrcMl.t | 23 +++++++- 4 files changed, 129 insertions(+), 39 deletions(-) create mode 100644 nix/srcml.nix diff --git a/README.md b/README.md index 5a4aff8e6..6afef6585 100644 --- a/README.md +++ b/README.md @@ -51,8 +51,8 @@ HTML views land in the sibling directory `../cregit-files/html`. See ## Preliminaries - Code is written in Scala, C++, Rust and Perl. -- Platform: Linux x86_64 or macOS arm64 — the pinned `srcml` 1.1.0 parser is a - prebuilt binary available only for those platforms. +- Platform: Linux x86_64 or macOS arm64 — `devenv` builds the pinned `srcml` + 1.1.0 parser from its release tag (`nix/srcml.nix`). ## Prerequisites diff --git a/devenv.nix b/devenv.nix index e229e15af..4fc05d163 100644 --- a/devenv.nix +++ b/devenv.nix @@ -1,42 +1,7 @@ { pkgs, ... }: let - # srcml is not in nixpkgs — package the official prebuilt binary. - # On feat/blob-exec-as-library we work on macOS arm64; on Linux x86_64 - # the upstream Linux binary still works. - srcml = - if pkgs.stdenv.hostPlatform.isDarwin then - pkgs.stdenv.mkDerivation { - pname = "srcml"; - version = "1.1.0"; - src = pkgs.fetchurl { - url = "https://github.com/srcML/srcML/releases/download/v1.1.0/srcml-1.1.0-macOS-arm64.tar.gz"; - sha256 = "1hxb6rzdbbfcqssirgzsrk81hcls4sjgdrz0zq34lar9v6n7zriq"; - }; - sourceRoot = "srcml-1.1.0-macOS-arm64"; - installPhase = '' - mkdir -p $out - cp -r usr/local/bin usr/local/lib usr/local/share $out/ - ''; - } - else - pkgs.stdenv.mkDerivation { - pname = "srcml"; - version = "1.1.0"; - src = pkgs.fetchurl { - url = "https://github.com/srcML/srcML/releases/download/v1.1.0/srcml_1.1.0-1_linux_amd64.tar.bz2"; - sha256 = "1hh9ii5fr4gv6xn81g99a9nr29x166hvmjkci4ygjriwkj63g7mz"; - }; - nativeBuildInputs = [ pkgs.autoPatchelfHook ]; - buildInputs = [ pkgs.stdenv.cc.cc.lib ]; - sourceRoot = "."; - installPhase = '' - mkdir -p $out/bin $out/lib $out/share - install -m755 bin/srcml $out/bin/ - cp -P lib/libsrcml.so.1 lib/libsrcml.so.1.1.0 $out/lib/ - cp -r share/. $out/share/ - ''; - }; + srcml = pkgs.callPackage ./nix/srcml.nix { }; EmailFind = pkgs.perlPackages.buildPerlPackage rec { pname = "Email-Find"; diff --git a/nix/srcml.nix b/nix/srcml.nix new file mode 100644 index 000000000..62250b5dc --- /dev/null +++ b/nix/srcml.nix @@ -0,0 +1,104 @@ +# srcML 1.1.0 built from its release tag, for the libsrcml headers. Later +# revisions change the tokens of files that 1.1.0 parses (srcML/srcML#2325 marks +# up some macro calls as expressions; UTF-8 input is no longer read as Latin-1). +{ lib +, stdenv +, fetchFromGitHub +, fetchurl +, cmake +, ninja +, jdk_headless +, libxml2 +, libxslt +, libarchive +, curl +, libiconv +}: + +let + antlr = fetchurl { + url = "https://www.antlr2.org/download/antlr-2.7.7.tar.gz"; + hash = "sha256-hTrrAhrvdYa9op50prAwBry1ZadVyGtmAy2Owxtn27k="; + }; + + tinysha1 = fetchFromGitHub { + owner = "mohaps"; + repo = "tinysha1"; + rev = "2795aa8de91b1797defdfbff61ed93b22b5ced81"; + hash = "sha256-oPZpqJKf6SHOD/bWG5IqtoODGlnQ9qsBiZ9FAY2Awd4="; + }; + + ctpl = fetchFromGitHub { + owner = "vit-vit"; + repo = "CTPL"; + rev = "437e135dbd94eb65b45533d9ce8ee28b5bd37b6d"; # tag v.0.0.2 + hash = "sha256-O9l7k1/2fVfEcyfJmWzXmju6TRQHK+NZFoaYmdd5KRg="; + }; + + cli11 = fetchurl { + url = "https://github.com/CLIUtils/CLI11/releases/download/v2.5.0/CLI11.hpp"; + hash = "sha256-S/CpSQqnIJF2zNpwVE+VQT5ZTSIHzKM8nNGN7RiaY6Y="; + }; +in +stdenv.mkDerivation { + pname = "srcml"; + version = "1.1.0"; + + src = fetchFromGitHub { + owner = "srcML"; + repo = "srcML"; + rev = "af1efb78ddd6fc4a8fc75bc6dc3c2e22c5fd812d"; # tag v1.1.0 + hash = "sha256-AdPVoUKI4/EigJz34BuGIbbN0jUGZTIxOQbmXr8IImM="; + }; + + nativeBuildInputs = [ cmake ninja jdk_headless ]; + + buildInputs = [ libxml2 libxslt libarchive curl ] + ++ lib.optionals stdenv.hostPlatform.isDarwin [ libiconv ]; + + # Without this the manpage step downloads a prebuilt page from a bare IP over + # plain HTTP, so $out carries no man page. + postPatch = '' + substituteInPlace doc/CMakeLists.txt \ + --replace-fail "add_subdirectory(manpage)" "# add_subdirectory(manpage)" + ''; + + # CMake edits the antlr and CTPL sources in place after fetching them, so each + # dependency needs a writable copy. + preConfigure = '' + deps="$NIX_BUILD_TOP/fetchcontent" + mkdir -p "$deps/cli11" "$deps/libarchive-include" + tar xf ${antlr} -C "$deps" + cp -r --no-preserve=mode,ownership ${tinysha1} "$deps/tinysha1" + cp -r --no-preserve=mode,ownership ${ctpl} "$deps/ctpl" + cp --no-preserve=mode,ownership ${cli11} "$deps/cli11/CLI11.hpp" + cp --no-preserve=mode,ownership \ + ${lib.getDev libarchive}/include/archive.h \ + ${lib.getDev libarchive}/include/archive_entry.h \ + "$deps/libarchive-include/" + + cmakeFlagsArray+=( + "-DFETCHCONTENT_SOURCE_DIR_ANTLRSRC=$deps/antlr-2.7.7" + "-DFETCHCONTENT_SOURCE_DIR_TINYSHA1=$deps/tinysha1" + "-DFETCHCONTENT_SOURCE_DIR_CTPL_STL_SRC=$deps/ctpl" + "-DFETCHCONTENT_SOURCE_DIR_CLI11=$deps/cli11" + "-DFETCHCONTENT_SOURCE_DIR_LIBARCHIVEINCLUDE1=$deps/libarchive-include" + "-DFETCHCONTENT_SOURCE_DIR_LIBARCHIVEINCLUDE=$deps/libarchive-include" + ) + ''; + + cmakeFlags = [ + (lib.cmakeBool "FETCHCONTENT_FULLY_DISCONNECTED" true) + (lib.cmakeBool "BUILD_CLIENT_TESTS" false) + ]; + + doCheck = false; + + meta = { + description = "Infrastructure for exploration, analysis and manipulation of source code"; + homepage = "https://www.srcML.org"; + license = lib.licenses.gpl3Only; + mainProgram = "srcml"; + platforms = [ "x86_64-linux" "aarch64-darwin" ]; + }; +} diff --git a/tokenize/t/tokenizeSrcMl.t b/tokenize/t/tokenizeSrcMl.t index 795e76189..0184c428c 100644 --- a/tokenize/t/tokenizeSrcMl.t +++ b/tokenize/t/tokenizeSrcMl.t @@ -18,7 +18,7 @@ plan skip_all => "ctags not on PATH" plan skip_all => "srcml2token not built (cd tokenize/srcMLtoken && make)" unless -x $srcml2token; -plan tests => 21; +plan tests => 24; # Exit status tokenizeSrcMl.pl uses for "srcML died / this tokenization is # unusable". Must match $PARSER_CRASH_EXIT there and BlobExec.ParserCrashExitCode. @@ -128,6 +128,27 @@ sub run_tokenizer { } } +{ + my $dies = "$workdir/srcml-that-dies"; + open(my $fh, '>', $dies) or die $!; + print $fh "#!/bin/sh\nkill -SEGV \$\$\n"; + close $fh; + chmod 0755, $dies or die $!; + + my $victim = "$workdir/ordinary.c"; + open($fh, '>', $victim) or die $!; + print $fh "int main() { return 0; }\n"; + close $fh; + + my ($status, $out, $err) = + run_tokenizer("--srcml='$dies'", "--position", "'$victim'"); + + is($status >> 8, $PARSER_CRASH_EXIT, + "a srcml killed by a signal exits $PARSER_CRASH_EXIT, whatever srcml we ship"); + is($out, "", "and emits no tokenization"); + like($err, qr/killed by signal 11/, "and names the signal on stderr"); +} + # The emptiness invariant the fix relies on, asserted rather than assumed: a # successful srcML tokenization is never empty. Even a zero-byte source file yields # the begin_unit/end_unit wrapper. That is what makes "empty stream" a safe failure From d28dcc3af6424eb71eb29d2a0098b9925a1ca665 Mon Sep 17 00:00:00 2001 From: Ellian Carlos Date: Sun, 4 Oct 2026 22:18:03 +0000 Subject: [PATCH 3/4] perf(tokenize): parse with libsrcml in srcml2token For each C, C++ or Java blob, tokenizeSrcMl.pl started bash, which ran `srcml ... | srcml2token`. Now srcml2token links libsrcml and parses the file itself: `srcml2token -l `. It makes the calls of `srcml -l --position ` (position on, tab stop 8, one unit, no hash, the default source encoding of 1.1.0) and feeds the XML to the same SAX handlers. Like the srcml command, it refuses a file extension that srcML does not know. srcml2token reads no XML any more, so it has one mode, and the srcml command is no longer run. tokenizeSrcMl.pl runs srcml2token directly. The bash pipeline, the PIPESTATUS file and the "unknown stage" case go. A signal death, a non-zero exit or an empty token stream still exit 33 (reason parser-crash); libsrcml 1.1.0 still crashes on the same inputs, now in the srcml2token process. --srcml is gone from tokenizeSrcMl.pl, tokenizerIdentity.pl and the callers. tokenize.pl still forwards it if given: editing tokenize.pl would change the identity of every extension, Rust too. The Makefile takes the libsrcml headers and library from SRCML_PREFIX, which devenv sets to the srcml derivation, and links with its rpath. Tokenizer identity: for C, C++ and Java it digests srcml2token, ctags and the libsrcml that `srcml2token --libsrcml-path` names (the library that is loaded), no longer the srcml binary. The value changes, so each project with cached C, C++ or Java rows stops with exit 3 until it is run with --retokenize. An old srcml2token does not answer --libsrcml-path, so the identity, and with it the run, fails first. Tokens: the same bytes and exit statuses as srcml 1.1.0 and the old srcml2token on 717 files (706 swept C, C++ and Java files and the fixtures), 3 of them srcML crashes in both. Tests: srcMLtoken's make test runs srcml2token on its five fixtures and checks --libsrcml-path and the unknown extension. tokenizeSrcMl.t checks the crash fixture and a stub srcml2token that kills itself. tokenizerIdentity.t covers the libsrcml component and an srcml2token without --libsrcml-path. --- README.md | 2 +- blobExec/shard_build.sh | 4 +- devenv.nix | 1 + run_pipeline_process.sh | 17 ++- test_retokenize_passthrough.sh | 3 + tests/test_token_worker.sh | 2 +- tokenize/srcMLtoken/Makefile | 36 +++---- tokenize/srcMLtoken/README.md | 26 +++-- tokenize/srcMLtoken/srcml2token.cpp | 156 ++++++++++++++------------- tokenize/t/tokenizeSrcMl.t | 25 ++--- tokenize/t/tokenizerIdentity.t | 31 ++++-- tokenize/tokenizeSrcMl.pl | 158 +++++----------------------- tokenize/tokenizerIdentity.pl | 31 ++++-- 13 files changed, 209 insertions(+), 283 deletions(-) diff --git a/README.md b/README.md index 6afef6585..19e0b9292 100644 --- a/README.md +++ b/README.md @@ -88,7 +88,7 @@ This builds, in dependency order: | artifact | module | toolchain | | ------------------------------------------------------------- | ------------------- | ------------------------- | -| `tokenize/srcMLtoken/srcml2token` | C++ transcoder | gcc + xerces-c | +| `tokenize/srcMLtoken/srcml2token` | srcML tokenizer | gcc + xerces-c + libsrcml | | `tokenize/rustTokenizer` binary | Rust tokenizer | cargo | | `blobExec/target/scala-2.13/blobExec-0.1.0-assembly.jar` | tokenization driver | sbt, JDK 21 | | `{slickGitLog,persons,remapCommits}/target/scala-2.10/*-one-jar.jar` | history / persons / remap tools | sbt 0.13, JDK 8 | diff --git a/blobExec/shard_build.sh b/blobExec/shard_build.sh index 7f13f0e3a..78ba1568c 100755 --- a/blobExec/shard_build.sh +++ b/blobExec/shard_build.sh @@ -88,9 +88,9 @@ if [ -z "$THREADS" ]; then fi # Default tokenizer dispatcher (srcML for .c/.h, rustTokenizer for .rs, …); the -# --srcml/--ctags paths are forwarded only to the srcML parser. +# --srcml2token/--ctags paths are forwarded only to the srcML parser. if [ -z "$TOK_CMD" ]; then - TOK_CMD="$CREGIT/tokenize/tokenize.pl --srcml2token=$CREGIT/tokenize/srcMLtoken/srcml2token --srcml=$(command -v srcml) --ctags=$(command -v ctags)" + TOK_CMD="$CREGIT/tokenize/tokenize.pl --srcml2token=$CREGIT/tokenize/srcMLtoken/srcml2token --ctags=$(command -v ctags)" fi export BFG_TOKENIZE_CMD="$TOK_CMD" JAVA="$(command -v java)" diff --git a/devenv.nix b/devenv.nix index 4fc05d163..4f3782fa8 100644 --- a/devenv.nix +++ b/devenv.nix @@ -69,4 +69,5 @@ in languages.rust.enable = true; env.LEGACY_JAVA_HOME = "${legacyJdk}"; + env.SRCML_PREFIX = "${srcml}"; } diff --git a/run_pipeline_process.sh b/run_pipeline_process.sh index a09827d8f..4c8ebb04a 100755 --- a/run_pipeline_process.sh +++ b/run_pipeline_process.sh @@ -1204,25 +1204,24 @@ if [ "$STEP_NUM" -ge "$FROM_STEP" ]; then export BFG_MEMO_DIR="$MEMO_DIR" # Route through the tokenize.pl dispatcher (not tokenizeSrcMl.pl directly) so it can -# fan out by language: srcML for .c/.h, rustTokenizer for .rs, etc. The --srcml* / -# --ctags paths are forwarded only to the srcML parser. Behavior-preserving for C. +# fan out by language: srcML for .c/.h, rustTokenizer for .rs, etc. The +# --srcml2token / --ctags paths are forwarded only to the srcML parser. export BFG_TOKENIZE_CMD="${CREGIT}/tokenize/tokenize.pl \ --srcml2token=${SRCML2TOKEN} \ - --srcml=$(which srcml) \ --ctags=$(which ctags)" # Which tokenizer produced which extension's tokens, so blobExec can refuse to -# reuse a cache built by a different one. Computed from the same three binaries -# BFG_TOKENIZE_CMD above pins, plus each language's parser and the dispatcher, by -# tokenize/tokenizerIdentity.pl. The failure it closes: `command` is the constant -# path tokenizeByBlobId/tokenBySha.pl and `mask` says which files, not how, so a -# rebuilt tokenizer moved neither and every cached row stayed a hit. +# reuse a cache built by a different one. Computed from the two binaries +# BFG_TOKENIZE_CMD above pins, the libsrcml that srcml2token loads, each +# language's parser and the dispatcher, by tokenize/tokenizerIdentity.pl. The +# failure it closes: `command` is the constant path tokenizeByBlobId/tokenBySha.pl +# and `mask` says which files, not how, so a rebuilt tokenizer moved neither and +# every cached row stayed a hit. # # Fatal if it cannot be computed. An empty identity would make the whole check a # no-op, and the pipeline would go back to reusing caches it cannot vouch for. TOKENIZER_IDENTITY=$(perl "${CREGIT}/tokenize/tokenizerIdentity.pl" \ --srcml2token="${SRCML2TOKEN}" \ - --srcml="$(which srcml)" \ --ctags="$(which ctags)") \ || die "cannot compute the tokenizer identity (tokenize/tokenizerIdentity.pl). blobExec needs it to tell a cache built by this tokenizer from one built by a diff --git a/test_retokenize_passthrough.sh b/test_retokenize_passthrough.sh index b604c212e..9bda1de17 100755 --- a/test_retokenize_passthrough.sh +++ b/test_retokenize_passthrough.sh @@ -98,6 +98,9 @@ repo_fixture() { echo "placeholder v1" > "$f/$p" chmod +x "$f/$p" done + # The identity also digests the libsrcml that srcml2token names. + printf '#!/bin/sh\necho "%s/libsrcml"\n' "$f" > "$f/tokenize/srcMLtoken/srcml2token" + echo "placeholder v1" > "$f/libsrcml" printf '%s' "$f" } diff --git a/tests/test_token_worker.sh b/tests/test_token_worker.sh index f3d794f87..9cbd26462 100755 --- a/tests/test_token_worker.sh +++ b/tests/test_token_worker.sh @@ -14,6 +14,6 @@ FIXTURES=("$ROOT"/tests/fixtures/* "$ROOT/tokenize/t/fixtures/srcml-position-cra python3 "$ROOT/tests/token_worker_driver.py" \ --worker "$ROOT/tokenizeByBlobId/tokenWorker.pl" \ --token-by-sha "$ROOT/tokenizeByBlobId/tokenBySha.pl" \ - --tokenize-command "$ROOT/tokenize/tokenize.pl --srcml2token=$SRCML2TOKEN --srcml=$(command -v srcml) --ctags=$(command -v ctags)" \ + --tokenize-command "$ROOT/tokenize/tokenize.pl --srcml2token=$SRCML2TOKEN --ctags=$(command -v ctags)" \ "${FIXTURES[@]/#/--fixture=}" \ --temp-root "$TEMP_ROOT" diff --git a/tokenize/srcMLtoken/Makefile b/tokenize/srcMLtoken/Makefile index 4458f33c0..9f34887bb 100644 --- a/tokenize/srcMLtoken/Makefile +++ b/tokenize/srcMLtoken/Makefile @@ -3,30 +3,30 @@ XERCES_CFLAGS ?= $(shell pkg-config --cflags xerces-c 2>/dev/null) XERCES_LIBS ?= $(shell pkg-config --libs xerces-c 2>/dev/null || echo -lxerces-c-3.2) +ifndef SRCML_PREFIX +$(error SRCML_PREFIX is not set: build inside `devenv shell`, or set it to the prefix of a srcML build with headers) +endif +SRCML_CFLAGS ?= -I$(SRCML_PREFIX)/include +SRCML_LIBS ?= -L$(SRCML_PREFIX)/lib -Wl,-rpath,$(SRCML_PREFIX)/lib -lsrcml + +FIXTURES = StringUtil.java:Java audit.c:C ceiling.cpp:C++ main.c:C srcml2tokenHandlers.cpp:C++ + default: srcml2token srcml2token: srcml2token.cpp srcml2tokenHandlers.cpp srcml2tokenHandlers.hpp srcml2token.hpp - g++ -g -std=c++11 -O -Wall -pedantic $(XERCES_CFLAGS) -o srcml2token srcml2token.cpp srcml2tokenHandlers.cpp $(XERCES_LIBS) + g++ -g -std=c++11 -O -Wall -pedantic $(XERCES_CFLAGS) $(SRCML_CFLAGS) -o srcml2token srcml2token.cpp srcml2tokenHandlers.cpp $(XERCES_LIBS) $(SRCML_LIBS) test: srcml2token mkdir -p tests/out - srcml --position --language Java tests/StringUtil.java | ./srcml2token > tests/out/StringUtil.java.token - diff tests/expected/StringUtil.java.token tests/out/StringUtil.java.token - srcml --position --language C tests/audit.c | ./srcml2token > tests/out/audit.c.token - diff tests/expected/audit.c.token tests/out/audit.c.token - srcml --position --language C++ tests/ceiling.cpp | ./srcml2token > tests/out/ceiling.cpp.token - diff tests/expected/ceiling.cpp.token tests/out/ceiling.cpp.token - srcml --position --language C tests/main.c | ./srcml2token > tests/out/main.c.token - diff tests/expected/main.c.token tests/out/main.c.token - srcml --position --language 'C++' tests/srcml2tokenHandlers.cpp | ./srcml2token > tests/out/srcml2tokenHandlers.cpp.token - diff tests/expected/srcml2tokenHandlers.cpp.token tests/out/srcml2tokenHandlers.cpp.token - + for f in $(FIXTURES); do \ + ./srcml2token -l "$${f#*:}" "tests/$${f%%:*}" > "tests/out/$${f%%:*}.token" \ + && diff "tests/expected/$${f%%:*}.token" "tests/out/$${f%%:*}.token" || exit 1; \ + done + test -f "$$(./srcml2token --libsrcml-path)" + printf 'int x;\n' > tests/out/unknown.xyz + ! ./srcml2token -l C tests/out/unknown.xyz 2> tests/out/unknown.err + grep -q 'does not know the extension' tests/out/unknown.err expected: srcml2token mkdir -p tests/expected - srcml --position --language 'C++' tests/srcml2tokenHandlers.cpp | ./srcml2token > tests/expected/srcml2tokenHandlers.cpp.token - srcml --position --language C tests/main.c | ./srcml2token > tests/expected/main.c.token - srcml --position --language 'C++' tests/ceiling.cpp | ./srcml2token > tests/expected/ceiling.cpp.token - srcml --position --language C tests/audit.c | ./srcml2token > tests/expected/audit.c.token - srcml --position --language Java tests/StringUtil.java | ./srcml2token > tests/expected/StringUtil.java.token - + for f in $(FIXTURES); do ./srcml2token -l "$${f#*:}" "tests/$${f%%:*}" > "tests/expected/$${f%%:*}.token"; done diff --git a/tokenize/srcMLtoken/README.md b/tokenize/srcMLtoken/README.md index 7146f42dc..1ea038fc6 100644 --- a/tokenize/srcMLtoken/README.md +++ b/tokenize/srcMLtoken/README.md @@ -1,31 +1,28 @@ # srcMLtoken -`srcMLtoken` transcodes the output of srcML ([www.srcml.org](www.srcml.org)) to a tokenized view that can be used by Cregit. - -## Requirements - -`srcml` must be installed and in the PATH ([www.srcml.org](www.srcml.org)) +`srcml2token` parses a C, C++ or Java file with libsrcml +([www.srcml.org](https://www.srcml.org)) and prints a tokenized view of its +srcML that Cregit uses. ## How to use -The input of `srml2token` is the output of `srcml`. `srml2token` takes no parameters, and it reads its input from _stdin_ and writes to _stdout_. - ```sh -srcml -l C --position | srcml2token +srcml2token -l C ``` +It parses the file as `srcml -l C --position ` does, and writes the +tokens to _stdout_. `srcml2token --libsrcml-path` prints the libsrcml that it +loads; `tokenize/tokenizerIdentity.pl` digests that file. + ## How to build -It requires `xerces` to be installed. Simply run: +It needs `xerces-c` and the libsrcml headers and library. Inside `devenv shell`, +`SRCML_PREFIX` names the srcML build of `nix/srcml.nix`. Run: ```sh -make +make && make test ``` -## TODO - -- Create a `cmake` configuration file. - ## License The code is mostly derived from the examples of Xerces, hence it is under the @@ -33,3 +30,4 @@ Apache-2.0 Dependencies: - `xerces`: Apache-2.0 + - `libsrcml`: GPL-3.0, so the linked `srcml2token` binary is under GPL-3.0 diff --git a/tokenize/srcMLtoken/srcml2token.cpp b/tokenize/srcMLtoken/srcml2token.cpp index a91f110ef..ca9d2951d 100644 --- a/tokenize/srcMLtoken/srcml2token.cpp +++ b/tokenize/srcMLtoken/srcml2token.cpp @@ -15,96 +15,106 @@ * limitations under the License. */ -// --------------------------------------------------------------------------- -// Includes -// --------------------------------------------------------------------------- -#include -#include -#include "srcml2token.hpp" +#include +#include +#include #include +#include "srcml2token.hpp" -// --------------------------------------------------------------------------- -// Local helper methods -// --------------------------------------------------------------------------- -void usage() +static int usage() { - XERCES_STD_QUALIFIER cout << "\nUsage:\n" - " srcml2token \n\n" - "This program converts the output of srcML into a simplified tokenized version\n" - << XERCES_STD_QUALIFIER endl; + XERCES_STD_QUALIFIER cerr << "Usage: srcml2token -l \n" + " srcml2token --libsrcml-path\n" + "Parses the file with libsrcml and prints its tokens.\n"; + return 2; } +static int fail(const std::string& why) +{ + XERCES_STD_QUALIFIER cerr << "srcml2token: " << why << XERCES_STD_QUALIFIER endl; + return 3; +} -// --------------------------------------------------------------------------- -// Program entry point -// --------------------------------------------------------------------------- -int main(int argC, char* argV[]) +// The tokenizer identity digests the library that is loaded, not the one on disk. +static int print_libsrcml_path() { - // Initialize the XML4C system - try - { - XMLPlatformUtils::Initialize(); - } + Dl_info info; + if (!dladdr(reinterpret_cast(&srcml_version_string), &info) || !info.dli_fname) + return fail("cannot find the loaded libsrcml"); + XERCES_STD_QUALIFIER cout << info.dli_fname << XERCES_STD_QUALIFIER endl; + return 0; +} - catch (const XMLException& toCatch) - { - XERCES_STD_QUALIFIER cerr << "Error during initialization! Message:\n" - << StrX(toCatch.getMessage()) << XERCES_STD_QUALIFIER endl; - return 1; - } +// The same calls as `srcml -l --position `. Like that command, +// refuse an extension that srcML does not know: it would ignore the language. +static int parse_source(const char* language, const char* filename, std::string& xml) +{ + srcml_archive* archive = srcml_archive_create(); + char* buffer = nullptr; + size_t size = 0; + int status = srcml_archive_write_open_memory(archive, &buffer, &size); + if (status == SRCML_STATUS_OK) status = srcml_archive_enable_option(archive, SRCML_OPTION_POSITION); + if (status == SRCML_STATUS_OK) status = srcml_archive_set_language(archive, language); + if (status == SRCML_STATUS_OK) status = srcml_archive_set_tabstop(archive, 8); + if (status == SRCML_STATUS_OK) status = srcml_archive_enable_solitary_unit(archive); + if (status == SRCML_STATUS_OK) status = srcml_archive_disable_hash(archive); + if (status == SRCML_STATUS_OK && !srcml_archive_check_extension(archive, filename)) + status = SRCML_STATUS_UNSET_LANGUAGE; + srcml_unit* unit = status == SRCML_STATUS_OK ? srcml_unit_create(archive) : nullptr; + if (unit) { + status = srcml_unit_set_language(unit, language); + if (status == SRCML_STATUS_OK) status = srcml_unit_set_filename(unit, filename); + if (status == SRCML_STATUS_OK) status = srcml_unit_parse_filename(unit, filename); + if (status == SRCML_STATUS_OK) status = srcml_archive_write_unit(archive, unit); + srcml_unit_free(unit); + } + srcml_archive_close(archive); + srcml_archive_free(archive); + if (status == SRCML_STATUS_OK && buffer) + xml.assign(buffer, size); + srcml_memory_free(buffer); + return status; +} +static int tokenize(const std::string& xml) +{ + XMLPlatformUtils::Initialize(); 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 exitCode = 0; + try { + MemBufInputSource source(reinterpret_cast(xml.data()), xml.size(), "srcml"); + parser->parse(source); + } + catch (const OutOfMemoryException&) { + XERCES_STD_QUALIFIER cerr << "OutOfMemoryException" << XERCES_STD_QUALIFIER endl; + exitCode = 4; + } + catch (const XMLException& e) { + XERCES_STD_QUALIFIER cerr << "\nError during parsing: \n" << StrX(e.getMessage()) << "\n" << XERCES_STD_QUALIFIER endl; + exitCode = 4; } - - // - // Delete the parser itself. Must be done prior to calling Terminate, below. - // delete parser; - XMLPlatformUtils::Terminate(); - - if (errorCount > 0) - return 4; - else - return 0; + return exitCode; } +int main(int argC, char* argV[]) +{ + if (argC == 2 && std::string(argV[1]) == "--libsrcml-path") + return print_libsrcml_path(); + if (argC != 4 || std::string(argV[1]) != "-l") + return usage(); + std::string xml; + int status = parse_source(argV[2], argV[3], xml); + if (status == SRCML_STATUS_UNSET_LANGUAGE) + return fail("srcML does not know the extension of [" + std::string(argV[3]) + "]"); + if (status != SRCML_STATUS_OK) + return fail("libsrcml could not parse [" + std::string(argV[3]) + "] as " + argV[2] + + " (status " + std::to_string(status) + ")"); + return tokenize(xml); +} diff --git a/tokenize/t/tokenizeSrcMl.t b/tokenize/t/tokenizeSrcMl.t index 0184c428c..d3c471d43 100644 --- a/tokenize/t/tokenizeSrcMl.t +++ b/tokenize/t/tokenizeSrcMl.t @@ -11,8 +11,6 @@ my $srcml2token = "$FindBin::Bin/../srcMLtoken/srcml2token"; my $fixtures = "$FindBin::Bin/../srcMLtoken/tests"; my $expected = "$FindBin::Bin/expected"; -plan skip_all => "srcml not on PATH" - unless system("srcml --version >/dev/null 2>&1") == 0; plan skip_all => "ctags not on PATH" unless system("ctags --version >/dev/null 2>&1") == 0; plan skip_all => "srcml2token not built (cd tokenize/srcMLtoken && make)" @@ -77,7 +75,7 @@ sub run_tokenizer { # -- the silent-empty defect ------------------------------------------------- # -# srcML 1.1.0 dies on a signal when --position is given on certain C/C++ inputs. +# libsrcml 1.1.0 dies on a signal when --position is given on certain C/C++ inputs. # This fixture is one of the 36 confirmed corpus files: the 1,263-byte # ext/liblzma/check/crc32_small.c from sumatrapdfreader__sumatrapdf, which is the # file that rules out any size explanation. Before the fix this exact input made @@ -92,7 +90,7 @@ sub run_tokenizer { # Guard the fixture itself: if a future srcML stops crashing on it, these # assertions would silently stop testing anything, so prove the crash is real # before asserting on how it is handled. - my $rawRc = system("srcml -l C --position '$crasher' >/dev/null 2>&1"); + my $rawRc = system("'$srcml2token' -l C '$crasher' >/dev/null 2>&1"); # A signal death reaches us in one of two encodings and both must be accepted: # as a raw wait status ($rc & 127) when the child is waited for directly, or as @@ -105,12 +103,12 @@ sub run_tokenizer { } SKIP: { - skip "srcml no longer dies on this fixture (rc=$rawRc): the upstream bug " + skip "srcml2token no longer dies on this fixture (rc=$rawRc): the upstream bug " . "appears fixed, so the crash-handling assertions cannot be exercised", 6 unless $rawSignal; is($rawSignal == 11 || $rawSignal == 6, 1, - "fixture kills srcml with SIGSEGV(11) or SIGABRT(6), got signal $rawSignal"); + "fixture kills srcml2token with SIGSEGV(11) or SIGABRT(6), got signal $rawSignal"); my ($status, $out, $err) = run_tokenizer("--position", "'$crasher'"); @@ -122,14 +120,13 @@ sub run_tokenizer { "no tokenization is emitted on stdout"); like($err, qr/FAILED/, "the failure is reported on stderr instead of passing silently"); - like($err, qr/killed by signal $rawSignal/, - "stderr names the signal, so srcml's death is attributed to srcml " - . "and not to srcml2token, which exits 0 on the truncated XML"); + like($err, qr/srcml2token was killed by signal $rawSignal/, + "stderr names the signal"); } } { - my $dies = "$workdir/srcml-that-dies"; + my $dies = "$workdir/srcml2token-that-dies"; open(my $fh, '>', $dies) or die $!; print $fh "#!/bin/sh\nkill -SEGV \$\$\n"; close $fh; @@ -141,10 +138,10 @@ sub run_tokenizer { close $fh; my ($status, $out, $err) = - run_tokenizer("--srcml='$dies'", "--position", "'$victim'"); + run_tokenizer("--srcml2token='$dies'", "--position", "'$victim'"); is($status >> 8, $PARSER_CRASH_EXIT, - "a srcml killed by a signal exits $PARSER_CRASH_EXIT, whatever srcml we ship"); + "a srcml2token killed by a signal exits $PARSER_CRASH_EXIT, whatever srcML we ship"); is($out, "", "and emits no tokenization"); like($err, qr/killed by signal 11/, "and names the signal on stderr"); } @@ -178,7 +175,7 @@ sub run_tokenizer { } # A path containing a single quote used to be interpolated straight into the shell -# command string. Now it is quoted, so it must tokenize rather than fail. +# command string, and broke the tokenizer. { my $odd = "$workdir/it's odd.c"; open(my $fh, '>', $odd) or die $!; @@ -186,6 +183,6 @@ sub run_tokenizer { close $fh; my ($status, $out, $err) = run_tokenizer("--position", "\"$odd\""); - is($status, 0, "a path containing a quote is passed to the shell safely"); + is($status, 0, "a path containing a quote tokenizes"); like($out, qr/begin_unit/, "and really was tokenized"); } diff --git a/tokenize/t/tokenizerIdentity.t b/tokenize/t/tokenizerIdentity.t index 1654eed50..f84b42679 100644 --- a/tokenize/t/tokenizerIdentity.t +++ b/tokenize/t/tokenizerIdentity.t @@ -51,8 +51,8 @@ sub fixture { write_file("$tok/rustTokenizer/target/release/rust_tokenizer", "RUST TOKENIZER v1\n"); make_path("$tok/m4Tokenizer"); write_file("$tok/m4Tokenizer/m4.py", "M4 v1\n"); - write_file("$dir/srcml2token", "SRCML2TOKEN v1\n"); - write_file("$dir/srcml", "SRCML v1\n"); + write_srcml2token($dir, "v1"); + write_file("$dir/libsrcml", "LIBSRCML v1\n"); write_file("$dir/ctags", "CTAGS v1\n"); return $dir; } @@ -65,11 +65,15 @@ sub write_file { chmod 0755, $path; } +sub write_srcml2token { + my ($dir, $version) = @_; + write_file("$dir/srcml2token", "#!/bin/sh\n# SRCML2TOKEN $version\necho '$dir/libsrcml'\n"); +} + sub identity { my ($dir, @extra) = @_; my @cmd = ("perl", "$dir/tokenize/tokenizerIdentity.pl", "--srcml2token=$dir/srcml2token", - "--srcml=$dir/srcml", "--ctags=$dir/ctags", @extra); open(my $ph, '-|', @cmd) or die "run: $!"; my $out = do { local $/; <$ph> }; @@ -126,12 +130,13 @@ for my $ext (grep { $_ ne 'rs' } sort keys %baseMap) { # --------------------------------------------------------------------------- # 3, the other direction — the srcML chain moves the srcML-routed extensions and -# not Rust. Each of the three binaries counts: the token stream is the product of -# all of them. -for my $component (qw(srcml2token srcml ctags)) { +# not Rust. Each of the three components counts: the token stream is the product +# of all of them. +for my $component (qw(srcml2token libsrcml ctags)) { my $d = fixture(); my ($before) = identity($d); - write_file("$d/$component", "$component v2\n"); + if ($component eq "srcml2token") { write_srcml2token($d, "v2") } + else { write_file("$d/$component", "$component v2\n") } my ($after) = identity($d); my %b = as_map($before); my %a = as_map($after); @@ -177,7 +182,7 @@ for my $shared (qw(tokenize.pl CregitLanguages.pm)) { my $d = fixture(); unlink("$d/tokenize/rustTokenizer/target/release/rust_tokenizer"); my @cmd = ("perl", "$d/tokenize/tokenizerIdentity.pl", - "--srcml2token=$d/srcml2token", "--srcml=$d/srcml", "--ctags=$d/ctags"); + "--srcml2token=$d/srcml2token", "--ctags=$d/ctags"); my $err = `@cmd 2>&1 1>/dev/null`; isnt($?, 0, "an unbuilt Rust tokenizer is fatal, not omitted"); like($err, qr/rust_tokenizer/, "and the message names the file"); @@ -185,12 +190,20 @@ for my $shared (qw(tokenize.pl CregitLanguages.pm)) { { my $d = fixture(); - my @cmd = ("perl", "$d/tokenize/tokenizerIdentity.pl", "--srcml=$d/srcml", "--ctags=$d/ctags"); + my @cmd = ("perl", "$d/tokenize/tokenizerIdentity.pl", "--ctags=$d/ctags"); my $err = `@cmd 2>&1 1>/dev/null`; isnt($?, 0, "omitting --srcml2token is fatal: .c tokens depend on it"); like($err, qr/--srcml2token/, "and the message names the flag"); } +{ + my $d = fixture(); + write_file("$d/srcml2token", "#!/bin/sh\nexit 0\n"); + my ($out, $status) = identity($d); + isnt($status, 0, "an srcml2token that does not name its libsrcml is fatal"); + is($out, "", "and prints no identity"); +} + # --------------------------------------------------------------------------- # --all-extensions covers the routed-but-unmasked ones (.am/.ac), so the flag can # be used the day the m4 lexer is fixed and M4 joins the mask. diff --git a/tokenize/tokenizeSrcMl.pl b/tokenize/tokenizeSrcMl.pl index 422489c28..101e9552c 100755 --- a/tokenize/tokenizeSrcMl.pl +++ b/tokenize/tokenizeSrcMl.pl @@ -15,14 +15,13 @@ use strict; use File::Basename; -use File::Temp qw(tempfile); use FindBin; use lib $FindBin::Bin; use CregitLanguages; -# Exit status meaning "this parse did not produce a usable tokenization": srcML -# died on a signal, either pipeline stage exited non-zero, or the token stream -# came back empty (which a healthy srcML parse never is -- see Tokenize). +# Exit status meaning "this parse did not produce a usable tokenization": +# srcml2token died on a signal or exited non-zero, or the token stream came back +# empty (which a healthy srcML parse never is -- see Verify_Parse). # # Picked to be distinguishable from every other outcome in the chain: below 128, # so it can never be confused with a shell's 128+signal encoding; clear of GNU @@ -51,7 +50,6 @@ Options: --srcml2token= - --srcml= --language= --ctags=- --position @@ -60,15 +58,13 @@ my $basedir = dirname($0); $basedir = "." if ($basedir eq ""); -my $srcml = "srcml"; my $srcml2token = "$basedir/srcMLtoken/srcml2token"; my $ctags = "ctags-universal"; my $language = ""; my $verbose; my $position = 0; -GetOptions ("srcml=s" => \$srcml, - "srcml2token=s" => \$srcml2token, +GetOptions ("srcml2token=s" => \$srcml2token, "language=s" => \$language, "ctags=s" => \$ctags, "position" => \$position, @@ -113,45 +109,14 @@ sub Tokenize { - my $saveDir = `pwd`; - chomp $saveDir; my ($filename) = @_; - - # The two stages still stream through a single pipe on purpose: buffering - # srcML's XML would cost memory proportional to the source (3.9MB of C becomes - # far more XML) and buy nothing. What changes is that the pipeline's per-stage - # exit statuses are no longer discarded. - # - # Why this is not just `close(parser)`: Perl's `open(FH, "cmd |")` reports - # failure only at close, and $? then carries the LAST command's status. Here - # that is srcml2token, which exits 0 even while printing - # "Fatal Error at file stdin, line 1, char 1 / invalid document structure" - # on the truncated XML a crashed srcML leaves behind. So an upstream srcML - # death was hidden twice over: once by open(), once by srcml2token's exit 0. - # - # bash's PIPESTATUS is the only thing that reports BOTH stages, so it is - # written to a temp file and read back after close. Note that PIPESTATUS - # encodes a signal death the way a shell does, as 128+signal (139 for SIGSEGV, - # 134 for SIGABRT), not as a raw wait status. - my ($statusFh, $statusFile) = - tempfile("cregit-pipestatus-XXXXXX", TMPDIR => 1, UNLINK => 1); - close $statusFh; - - my $pipeline = sprintf( - '%s -l %s --position %s | %s; printf %%s\\ %%s "${PIPESTATUS[0]}" "${PIPESTATUS[1]}" > %s', - Shell_Quote($srcml), Shell_Quote($language), - Shell_Quote($filename), Shell_Quote($srcml2token), - Shell_Quote($statusFile)); - - # Explicitly bash, not the `sh` that a one-argument open() would pick: - # PIPESTATUS is a bashism and /bin/sh is not guaranteed to be bash. - open(parser, "-|", "bash", "-c", $pipeline) - or die "Unable to execute srcml pipeline on file [$filename]: $!"; + open(my $parser, "-|", $srcml2token, "-l", $language, $filename) + or die "Unable to execute srcml2token on file [$filename]: $!"; my $lastLine = -1; my $tokensRead = 0; - while () { + while (<$parser>) { $tokensRead++; # print STDERR; chomp; @@ -184,89 +149,29 @@ sub Tokenize } } - my $closed = close parser; - my $closeStatus = $?; - chdir($saveDir); - - Verify_Parse($filename, $tokensRead, $statusFile, $closed, $closeStatus); -} - -# Single-quote a string for the shell: end the quote, escape the literal quote, -# reopen. Replaces the bare '$filename' interpolation that used to sit in the -# command string, which broke on any path containing a quote. -sub Shell_Quote -{ - my ($s) = @_; - $s = '' unless defined $s; - $s =~ s/'/'\\''/g; - return "'$s'"; + close $parser; + Verify_Parse($filename, $tokensRead, $?); } -# Fail closed. Anything other than "both stages exited 0 and we got a non-empty -# token stream" is a defect, and must be reported with a status the caller can -# count rather than silently becoming a 0-byte tokenization. +# Fail closed. Anything other than "srcml2token exited 0 with a non-empty token +# stream" is a defect, and must be reported with a status the caller can count +# rather than silently becoming a 0-byte tokenization. sub Verify_Parse { - my ($filename, $tokensRead, $statusFile, $closed, $closeStatus) = @_; - - my ($srcmlStatus, $tokenStatus) = Read_Pipe_Status($statusFile); - - # srcML first: it is the stage that crashes, and its status is the one the old - # code could never see. - if (defined $srcmlStatus and $srcmlStatus > 128) { - my $signal = $srcmlStatus - 128; - Parser_Crash($filename, - "srcml was killed by signal $signal (shell status $srcmlStatus). " - . "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 (defined $srcmlStatus and $srcmlStatus != 0) { - Parser_Crash($filename, "srcml exited $srcmlStatus."); - } - if (defined $tokenStatus and $tokenStatus > 128) { - my $signal = $tokenStatus - 128; - Parser_Crash($filename, - "srcml2token was killed by signal $signal (shell status $tokenStatus)."); - } - if (defined $tokenStatus and $tokenStatus != 0) { - Parser_Crash($filename, "srcml2token exited $tokenStatus."); - } - - # If bash never wrote the statuses we cannot claim the parse was clean, so the - # close status is used as a (weaker) backstop rather than ignored. - if (not defined $srcmlStatus or not defined $tokenStatus) { - if (not $closed or $closeStatus != 0) { - Parser_Crash_Unknown($filename, $closeStatus); - } - } - - # The emptiness invariant, and the reason a crash can be caught even without - # PIPESTATUS: a successful srcML tokenization is NEVER empty. Even a zero-byte - # source file yields the begin_unit/end_unit wrapper (measured: 80 bytes, and - # byte-identical for an empty and a whitespace-only file). So zero tokens here - # always means the parse failed -- unlike a general-purpose filter, where empty - # output from empty input is legitimate. - if ($tokensRead == 0) { - Parser_Crash($filename, - "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."); - } -} - -# Reads the two shell statuses bash left behind. Returns (undef, undef) when the -# file is missing or unparseable, so the caller can fall back rather than assume 0. -sub Read_Pipe_Status -{ - my ($statusFile) = @_; - open(my $fh, '<', $statusFile) or return (undef, undef); - my $line = <$fh>; - close $fh; - return (undef, undef) unless defined $line; - chomp $line; - return (undef, undef) unless $line =~ /^([0-9]+)\s+([0-9]+)$/; - return ($1, $2); + my ($filename, $tokensRead, $status) = @_; + my $signal = $status & 127; + Parser_Crash($filename, + "srcml2token was killed by signal $signal. srcML 1.1.0, which it links, " + . "dies on a signal on some C/C++ inputs under --position.") if $signal; + Parser_Crash($filename, "srcml2token exited " . ($status >> 8) . ".") if $status != 0; + + # Even a zero-byte source file yields the begin_unit/end_unit wrapper, so zero + # tokens always means the parse failed. + Parser_Crash($filename, + "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.") + if $tokensRead == 0; } sub Parser_Crash @@ -278,17 +183,6 @@ sub Parser_Crash exit($PARSER_CRASH_EXIT); } -sub Parser_Crash_Unknown -{ - my ($filename, $closeStatus) = @_; - print STDERR "cregit: tokenization of [$filename] FAILED: the srcml pipeline " - . "reported a non-zero close status ($closeStatus) and bash did not report " - . "PIPESTATUS, so the failing stage is unknown.\n"; - exit($PARSER_CRASH_EXIT); -} - - - sub Declarations_Test { foreach my $line (sort {$a <=> $b} keys %declarations) { diff --git a/tokenize/tokenizerIdentity.pl b/tokenize/tokenizerIdentity.pl index cdee7f0e7..2d4471c38 100755 --- a/tokenize/tokenizerIdentity.pl +++ b/tokenize/tokenizerIdentity.pl @@ -32,10 +32,10 @@ # * its language's parser (CregitLanguages::LANG_PARSER_REL) — the tokenizer # itself. For Rust that is the compiled cargo artifact, which is exactly the # file that went stale. -# * srcml2token, srcml and ctags, for the srcML-routed languages only. The -# token stream is the product of all three: srcml2token transcodes srcML's -# XML, and tokenizeSrcMl.pl merges ctags output into it. A change in any of -# them changes the tokens. +# * srcml2token, the libsrcml it loads, and ctags, for the srcML-routed +# languages only. srcml2token parses with libsrcml, and tokenizeSrcMl.pl +# merges ctags output into its tokens. A change in any of them changes the +# tokens. # * tokenize.pl and CregitLanguages.pm, for every extension. tokenize.pl is # the dispatcher that decides which parser runs and with which flags # (--position among them, which the token format depends on), and @@ -69,21 +69,18 @@ ext=,ext=,... Options: - --srcml2token= the built C++ transcoder (required for C/C++/Java) - --srcml= the srcml binary (required for C/C++/Java) + --srcml2token= the built srcML tokenizer (required for C/C++/Java) --ctags= the ctags binary (required for C/C++/Java) --all-extensions cover every extension in the table, not only the masked ones (.am/.ac are routed but not masked) "; my $srcml2tokenPath = ""; -my $srcmlPath = ""; my $ctagsPath = ""; my $allExtensions = 0; GetOptions( "srcml2token=s" => \$srcml2tokenPath, - "srcml=s" => \$srcmlPath, "ctags=s" => \$ctagsPath, "all-extensions" => \$allExtensions, ) or die($usage); @@ -119,6 +116,19 @@ sub digest_of { return $sha->hexdigest; } +sub libsrcml_of { + my ($srcml2token) = @_; + open(my $ph, '-|', $srcml2token, "--libsrcml-path") + or die "tokenizerIdentity: cannot run [$srcml2token]: $!\n"; + my $path = <$ph> // ""; + close $ph; + chomp $path; + die "tokenizerIdentity: [$srcml2token] does not answer --libsrcml-path, so it is\n" + . " not the srcml2token that parses with libsrcml. Rebuild it: make -C tokenize/srcMLtoken\n" + if $? != 0 or $path eq ""; + return $path; +} + my %digestCache; sub cached_digest { my ($path) = @_; @@ -127,6 +137,7 @@ sub cached_digest { } my @pairs; +my $libsrcml; for my $ext (@extensions) { my $language = $CregitLanguages::EXT_LANG{$ext}; die "tokenizerIdentity: no language for extension [$ext]\n" unless defined $language; @@ -135,12 +146,10 @@ sub cached_digest { my @components = (@shared, $parser); - # The srcML chain is three binaries deep and all three shape the tokens. # Required rather than optional: an identity for .c computed without # srcml2token would not move when srcml2token does. if ($CregitLanguages::LANG_PARSER_REL{$language} eq $CregitLanguages::SRCML_PARSER_REL) { for my $pair (["--srcml2token", $srcml2tokenPath], - ["--srcml", $srcmlPath], ["--ctags", $ctagsPath]) { my ($flag, $path) = @$pair; die "tokenizerIdentity: $flag is required, because extension [$ext] is parsed by\n" @@ -148,6 +157,8 @@ sub cached_digest { if $path eq ""; push @components, $path; } + $libsrcml //= libsrcml_of($srcml2tokenPath); + push @components, $libsrcml; } # Digest of the component digests, not of the concatenated bytes: the list is From 22c9a075602fa70a7550c72ff35e5b9cf23c0399 Mon Sep 17 00:00:00 2001 From: Ellian Carlos Date: Sun, 4 Oct 2026 22:22:49 +0000 Subject: [PATCH 4/4] perf(tokenize): run the srcML chain in the worker For C, C++ and Java the worker from the pool commit still starts tokenize.pl, tokenizeSrcMl.pl, ctags and srcml2token for each blob. Now tokenize/CregitSrcMl.pm does the work of tokenizeSrcMl.pl in the worker process. It keeps one `ctags --_interactive` per language alive and starts only srcml2token for each file, in its own process group, so a timeout kills it. Other languages, and a BFG_TOKENIZE_CMD that is not tokenize.pl with only --srcml2token, --ctags and --position, still start the command for each blob. ctags runs with the -x format of tokenizeSrcMl.pl, not with JSON: in JSON, ctags drops a field that is not valid UTF-8, so a Latin-1 signature or a truncated UTF-8 name lost its DECL line. The worker does not start if srcml2token does not answer --libsrcml-path, so an old srcml2token cannot make every srcML blob a parser crash. Tokenizer identity: CregitSrcMl.pm joins the srcML components. The previous commit already changes the identity of these extensions, so this costs no second --retokenize. Tokens: on the 720 swept files the worker gives the bytes and exit statuses of the default path, which are those of srcml 1.1.0 and the old srcml2token. Tests: test_token_worker.sh gets three fixtures with non-ASCII names (UTF-8 C and Java, Latin-1 C), a hung and a crashing srcml2token in the worker, and the old-srcml2token check. tokenizerIdentity.t checks that editing CregitSrcMl.pm moves the .c identity and not .rs. --- tests/fixtures/Utf8.java | 3 + tests/fixtures/latin1.c | 1 + tests/fixtures/utf8.c | 2 + tests/token_worker_driver.py | 46 ++++++ tokenize/CregitSrcMl.pm | 225 ++++++++++++++++++++++++++++ tokenize/t/tokenizerIdentity.t | 10 +- tokenize/tokenizerIdentity.pl | 10 +- tokenizeByBlobId/WORKER_PROTOCOL.md | 12 +- tokenizeByBlobId/tokenWorker.pl | 30 +++- 9 files changed, 323 insertions(+), 16 deletions(-) create mode 100644 tests/fixtures/Utf8.java create mode 100644 tests/fixtures/latin1.c create mode 100644 tests/fixtures/utf8.c create mode 100644 tokenize/CregitSrcMl.pm diff --git a/tests/fixtures/Utf8.java b/tests/fixtures/Utf8.java new file mode 100644 index 000000000..f6b8783b3 --- /dev/null +++ b/tests/fixtures/Utf8.java @@ -0,0 +1,3 @@ +class Café { + int grüß(String ä) { return 1; } +} diff --git a/tests/fixtures/latin1.c b/tests/fixtures/latin1.c new file mode 100644 index 000000000..4191f97fb --- /dev/null +++ b/tests/fixtures/latin1.c @@ -0,0 +1 @@ +int h(int bé) { return 2; } diff --git a/tests/fixtures/utf8.c b/tests/fixtures/utf8.c new file mode 100644 index 000000000..a5d166fda --- /dev/null +++ b/tests/fixtures/utf8.c @@ -0,0 +1,2 @@ +int café(int ä) { return 0; } +int g(int 😀x) { return 1; } diff --git a/tests/token_worker_driver.py b/tests/token_worker_driver.py index 2a270f1e5..a7b94f943 100755 --- a/tests/token_worker_driver.py +++ b/tests/token_worker_driver.py @@ -5,6 +5,7 @@ import hashlib import os import pathlib +import shutil import subprocess import time @@ -119,6 +120,49 @@ def check_timeout(args, marker): worker.close() +def in_process_command(args, marker): + options = args.tokenize_command.split() + real = next(o.split("=", 1)[1] for o in options if o.startswith("--srcml2token=")) + srcml2token = write_script( + args.temp_root / "srcml2token.sh", + 'src="${@: -1}"\n' + f'grep -qs HANG_MARKER "$src" && exec perl -e "sleep 100" {marker}\n' + 'grep -qs ABORT_MARKER "$src" && kill -ABRT $$\n' + f'exec {real} "$@"\n', + ) + ctags = write_script( + args.temp_root / "ctags.sh", f'echo CTAGS_STDERR >&2\nexec {shutil.which("ctags")} "$@"\n' + ) + options = [o for o in options if not o.startswith(("--srcml2token=", "--ctags="))] + return " ".join(options + [f"--srcml2token={srcml2token}", f"--ctags={ctags}"]) + + +def check_in_process_failures(args, marker): + memo = new_dir(args, "in-process-memo") + worker = Worker(args.worker, memo, in_process_command(args, marker)) + (exit_code, _, _), elapsed = timed_request(worker, "hang.c", b"int HANG_MARKER;\n") + check(exit_code == TIMEOUT_EXIT and elapsed < 3, f"in process, a hung srcml2token exits {TIMEOUT_EXIT}") + check(no_process_left(marker), "and is killed") + + exit_code, out, err = worker.request("abort.c", b"int ABORT_MARKER;\n") + check(exit_code == PARSER_CRASH_EXIT and out == b"", f"in process, a srcml2token signal death exits {PARSER_CRASH_EXIT}") + check(b"killed by signal 6" in err and b"CTAGS_STDERR" in err, "with the srcml2token and ctags stderr") + check(not any(memo.rglob("*")), "and nothing is memoized") + + fixture = args.fixture[0] + expected = token_by_sha(args, new_dir(args, "in-process-reference"), fixture).stdout + check(worker.request(fixture.name, fixture.read_bytes())[1] == expected, "the worker recovers") + worker.close() + + +def check_stale_srcml2token(args): + stale = write_script(args.temp_root / "srcml2token", "exit 0\n") + command = f"{args.tokenize_command} --srcml2token={stale}" + env = dict(os.environ, BFG_MEMO_DIR=str(new_dir(args, "stale-memo")), BFG_TOKENIZE_CMD=command) + started = subprocess.run([str(args.worker)], input=b"", capture_output=True, env=env) + check(started.returncode != 0 and b"READY" not in started.stdout, "a srcml2token without --libsrcml-path stops the start") + + def main(): parser = argparse.ArgumentParser() parser.add_argument("--worker", required=True, type=pathlib.Path) @@ -131,6 +175,8 @@ def main(): marker = f"cregit-token-worker-{os.getpid()}" check_same_as_token_by_sha(args) check_timeout(args, marker) + check_in_process_failures(args, marker) + check_stale_srcml2token(args) print("ALL TOKEN WORKER TESTS PASSED") diff --git a/tokenize/CregitSrcMl.pm b/tokenize/CregitSrcMl.pm new file mode 100644 index 000000000..0b61382d7 --- /dev/null +++ b/tokenize/CregitSrcMl.pm @@ -0,0 +1,225 @@ +package CregitSrcMl; + +# tokenizeSrcMl.pl in a persistent process, with one ctags per language. Its +# output must stay byte-identical to tokenizeSrcMl.pl (tests/test_token_worker.sh). + +use strict; +use warnings; +use bytes; +use File::Temp (); +use POSIX qw(_exit); + +our $PARSER_CRASH_EXIT = 33; + +my $EMPTY_STREAM = "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."; + +sub new { + my ($class, %options) = @_; + my $self = bless { ctags => "ctags-universal", position => 0, %options, + ctags_procs => {}, retired_errors => [] }, $class; + open(my $probe, '-|', $self->{srcml2token}, "--libsrcml-path") + or die "unable to execute srcml2token [$self->{srcml2token}]: $!\n"; + my $library = <$probe> // ""; + close($probe); + die "[$self->{srcml2token}] does not answer --libsrcml-path; rebuild it: make -C tokenize/srcMLtoken\n" + if $? != 0 or $library eq ""; + return $self; +} + +sub tokenize { + my ($self, $language, $inputName, $workDir, $timeoutSecs) = @_; + my $timedOut = 0; + local $SIG{ALRM} = sub { $timedOut = 1; $self->kill_all() }; + alarm($timeoutSecs); + my @result = eval { $self->run($language, $inputName, $workDir) }; + my $error = $@; + alarm(0); + my $helperErr = $self->take_helper_stderr(); + + if ($timedOut) { + $self->shutdown(); + return (124, "", $helperErr . "tokenize command timed out after [$timeoutSecs] seconds\n"); + } + return (255, "", $helperErr . $error) if not @result; + $result[2] = $helperErr . $result[2]; + return @result; +} + +sub run { + my ($self, $language, $inputName, $workDir) = @_; + my $declarations = $self->read_declarations($language, $inputName, "$workDir/$inputName"); + my ($status, $tokens, $stderr) = $self->run_srcml2token($language, $inputName, $workDir); + return parser_crash($inputName, $stderr, srcml2token_failure($status)) if $status != 0; + return parser_crash($inputName, $stderr, $EMPTY_STREAM) if not @$tokens; + return (0, merge($tokens, $declarations, $self->{position}), $stderr); +} + +sub srcml2token_failure { + my ($status) = @_; + my $signal = $status & 127; + return "srcml2token exited " . ($status >> 8) . "." if not $signal; + return "srcml2token was killed by signal $signal. srcML 1.1.0, which it links, " + . "dies on a signal on some C/C++ inputs under --position."; +} + +sub parser_crash { + my ($filename, $stderr, $why) = @_; + return ($PARSER_CRASH_EXIT, "", $stderr + . "cregit: tokenization of [$filename] FAILED: $why\n" + . "cregit: refusing to emit a tokenization for [$filename]; " + . "exiting $PARSER_CRASH_EXIT so the caller can count this blob.\n"); +} + +sub merge { + my ($tokens, $declarations, $position) = @_; + my $lastLine = -1; + my $output = ""; + for my $tokenLine (@$tokens) { + die "unable to parse srcml line [$tokenLine]\n" unless $tokenLine =~ /^([0-9]+|-):([0-9]+|-)\s+(.+)$/; + my ($line, $col, $token) = ($1, $2, $3); + my $lineNumber = $line eq '-' ? 0 : $line; + $output .= declaration_lines($declarations->{$line}, $line, $position) if $lineNumber != $lastLine; + $lastLine = $lineNumber; + $output .= "$line:$col|" if $position; + $output .= "$token\n"; + $output .= ($position ? "-:-|" : "") . "\n" if $token =~ /^end_/; + } + return $output; +} + +# Like tokenizeSrcMl.pl, a name that is twice on one line prints the type of its +# last declaration twice. +sub declaration_lines { + my ($declarations, $line, $position) = @_; + return "" if not $declarations; + return join "", map { ($position ? "$line:-|" : "") . "DECL|$declarations->{types}{$_}|$_\n" } + @{ $declarations->{names} }; +} + +sub read_declarations { + my ($self, $language, $inputName, $path) = @_; + open(my $in, '<:raw', $path) or die "unable to read [$path]: $!\n"; + my $source = do { local $/; <$in> } // ""; + my $ctags = $self->ctags_for($language); + print {$ctags->{in}} qq({"command":"generate-tags","filename":"$inputName","size":) + . length($source) . "}\n", $source; + $ctags->{in}->flush; + + my %declarations; + while (defined(my $tag = readline($ctags->{out}))) { + return \%declarations if $tag =~ /\A\{"_type": "completed"/; + chomp $tag; + if ($tag !~ /^([0-9]+) (.+) @@@ (.*) @@@ (.*)$/) { + $self->retire(delete $self->{ctags_procs}{$language}); + die "unable to parse output [$tag]\n"; + } + my ($line, $name, $type, $signature) = ($1, $2, $3, $4); + $name .= " $signature" if $signature ne "-"; + $declarations{$line}{types}{$name} = $type; + push @{ $declarations{$line}{names} }, $name; + } + $self->retire(delete $self->{ctags_procs}{$language}); + return \%declarations; +} + +# The same -x format as tokenizeSrcMl.pl, so that the names are byte-identical: +# ctags --_interactive drops a JSON field that is not valid UTF-8. +sub ctags_for { + my ($self, $language) = @_; + return $self->{ctags_procs}{$language} //= do { + my $proc = spawn_pipe($self->{ctags}, "--_interactive", "--language-force=$language", "-x", + "--sort=n", "--_xformat=%n %N @@@ %K @@@ %S"); + my $hello = readline($proc->{out}) // "EOF"; + die "ctags did not start: $hello\n" unless $hello =~ /"_type": "program"/; + $proc; + }; +} + +sub run_srcml2token { + my ($self, $language, $inputName, $workDir) = @_; + my $out = File::Temp->new(TEMPLATE => "cregit-srcml2token-out-XXXXX", TMPDIR => 1); + my $err = File::Temp->new(TEMPLATE => "cregit-srcml2token-err-XXXXX", TMPDIR => 1); + my $pid = fork() // die "unable to fork srcml2token: $!\n"; + if ($pid == 0) { + setpgrp(0, 0); + chdir($workDir) and open(STDIN, '<', '/dev/null') and open(STDOUT, '>', $out->filename) + and open(STDERR, '>', $err->filename) or _exit(255); + { no warnings 'exec'; exec $self->{srcml2token}, "-l", $language, $inputName; } + print STDERR "unable to execute srcml2token [$self->{srcml2token}]: $!\n"; + _exit(255); + } + $self->{parser_pid} = $pid; + waitpid($pid, 0); + my $status = $?; + delete $self->{parser_pid}; + seek($out, 0, 0); + chomp(my @tokens = <$out>); + return ($status, \@tokens, read_all($err)); +} + +# A helper writes its stderr to a file: a full stderr pipe would block it while +# this process waits for its stdout. +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 ($pid == 0) { + open(STDIN, '<&', $childIn) and open(STDOUT, '>&', $childOut) + and open(STDERR, '>>', $err->filename) or _exit(255); + { 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, err => $err }; +} + +sub take_helper_stderr { + my ($self) = @_; + my @live = grep { $_ } values(%{ $self->{ctags_procs} }); + my $text = join "", map { read_all($_) } @{ $self->{retired_errors} }, map { $_->{err} } @live; + $self->{retired_errors} = []; + truncate($_->{err}, 0) for @live; + return $text; +} + +sub read_all { + my ($fh) = @_; + seek($fh, 0, 0); + local $/; + return <$fh> // ""; +} + +sub retire { + my ($self, $proc) = @_; + return if not $proc; + push @{ $self->{retired_errors} }, $proc->{err}; + close($proc->{in}); + close($proc->{out}); + kill 'KILL', $proc->{pid}; + waitpid($proc->{pid}, 0); +} + +sub kill_all { + my ($self) = @_; + if (my $pid = $self->{parser_pid}) { + kill 'TERM', -$pid; + select(undef, undef, undef, 0.1); + kill 'KILL', -$pid; + } + kill 'KILL', map { $_->{pid} } grep { $_ } values(%{ $self->{ctags_procs} }); +} + +sub shutdown { + my ($self) = @_; + $self->retire(delete $self->{ctags_procs}{$_}) for keys %{ $self->{ctags_procs} }; +} + +1; diff --git a/tokenize/t/tokenizerIdentity.t b/tokenize/t/tokenizerIdentity.t index f84b42679..078add30f 100644 --- a/tokenize/t/tokenizerIdentity.t +++ b/tokenize/t/tokenizerIdentity.t @@ -40,7 +40,7 @@ sub fixture { my $dir = tempdir(CLEANUP => 1); my $tok = "$dir/tokenize"; make_path($tok); - for my $f (qw(tokenize.pl CregitLanguages.pm tokenizerIdentity.pl tokenizeSrcMl.pl)) { + for my $f (qw(tokenize.pl CregitLanguages.pm tokenizerIdentity.pl tokenizeSrcMl.pl CregitSrcMl.pm)) { copy("$realTokenizeDir/$f", "$tok/$f") or die "copy $f: $!"; } chmod 0755, "$tok/tokenizerIdentity.pl", "$tok/tokenize.pl", "$tok/tokenizeSrcMl.pl"; @@ -144,15 +144,15 @@ for my $component (qw(srcml2token libsrcml ctags)) { is($a{rs}, $b{rs}, "changing $component leaves .rs alone"); } -# tokenizeSrcMl.pl itself, for completeness. -{ +# tokenizeSrcMl.pl itself, and CregitSrcMl.pm, its copy in the tokenizer worker. +for my $script (qw(tokenizeSrcMl.pl CregitSrcMl.pm)) { my $d = fixture(); my ($before) = identity($d); - write_file("$d/tokenize/tokenizeSrcMl.pl", "# v2\n"); + write_file("$d/tokenize/$script", "# v2\n"); my ($after) = identity($d); my %b = as_map($before); my %a = as_map($after); - isnt($a{c}, $b{c}, "editing tokenizeSrcMl.pl moves the .c identity"); + isnt($a{c}, $b{c}, "editing $script moves the .c identity"); is($a{rs}, $b{rs}, "and leaves .rs alone"); } diff --git a/tokenize/tokenizerIdentity.pl b/tokenize/tokenizerIdentity.pl index 2d4471c38..62898711d 100755 --- a/tokenize/tokenizerIdentity.pl +++ b/tokenize/tokenizerIdentity.pl @@ -32,10 +32,10 @@ # * its language's parser (CregitLanguages::LANG_PARSER_REL) — the tokenizer # itself. For Rust that is the compiled cargo artifact, which is exactly the # file that went stale. -# * srcml2token, the libsrcml it loads, and ctags, for the srcML-routed -# languages only. srcml2token parses with libsrcml, and tokenizeSrcMl.pl -# merges ctags output into its tokens. A change in any of them changes the -# tokens. +# * srcml2token, the libsrcml it loads, ctags and CregitSrcMl.pm, for the +# srcML-routed languages only. srcml2token parses with libsrcml, and +# tokenizeSrcMl.pl (or CregitSrcMl.pm in the tokenizer worker) merges ctags +# output into its tokens. A change in any of them changes the tokens. # * tokenize.pl and CregitLanguages.pm, for every extension. tokenize.pl is # the dispatcher that decides which parser runs and with which flags # (--position among them, which the token format depends on), and @@ -158,7 +158,7 @@ sub cached_digest { push @components, $path; } $libsrcml //= libsrcml_of($srcml2tokenPath); - push @components, $libsrcml; + push @components, $libsrcml, "$basedir/CregitSrcMl.pm"; } # Digest of the component digests, not of the concatenated bytes: the list is diff --git a/tokenizeByBlobId/WORKER_PROTOCOL.md b/tokenizeByBlobId/WORKER_PROTOCOL.md index 1b5c4d200..d416f29dd 100644 --- a/tokenizeByBlobId/WORKER_PROTOCOL.md +++ b/tokenizeByBlobId/WORKER_PROTOCOL.md @@ -8,6 +8,14 @@ The worker reads `BFG_MEMO_DIR` and `BFG_TOKENIZE_CMD` from its environment, as `tokenBySha.pl` does. Its tokens, its memo files and its exit statuses are the same as those of `tokenBySha.pl`. +For C, C++ and Java, the worker does the work of `tokenizeSrcMl.pl` in its own +process (`tokenize/CregitSrcMl.pm`) if `BFG_TOKENIZE_CMD` is `tokenize.pl` with +only the options `--srcml2token=`, `--ctags=` and `--position`. It keeps one +`ctags --_interactive` for each language alive, and starts only `srcml2token` +for each file. For all other languages and commands, it starts +`BFG_TOKENIZE_CMD` for each blob. The worker does not start if `srcml2token` +does not answer `--libsrcml-path`. + ## Messages At start, the worker writes `READY\n` to stdout. Then, for each request: @@ -26,8 +34,8 @@ worker -> JVM: RES \n ## Failures -- The worker runs the tokenizer in its own process group. At the timeout it - kills the group and answers `124`. +- The worker runs the tokenizer (or `srcml2token`) in its own process group. At the + timeout it kills the group and the helpers, and answers `124`. - If no answer comes in `timeoutSeconds + 5` seconds, blobExec kills the worker and its children, starts a new worker, and reports a timeout. - If the worker stops in a request, blobExec reports a tokenizer error for diff --git a/tokenizeByBlobId/tokenWorker.pl b/tokenizeByBlobId/tokenWorker.pl index 34c4de2ae..f44c708e8 100755 --- a/tokenizeByBlobId/tokenWorker.pl +++ b/tokenizeByBlobId/tokenWorker.pl @@ -1,13 +1,13 @@ #!/usr/bin/env perl # A persistent tokenBySha.pl with the same output and memo. The protocol is in -# WORKER_PROTOCOL.md. +# WORKER_PROTOCOL.md. C, C++ and Java go through CregitSrcMl in this process. use strict; use warnings; use bytes; use Digest::SHA qw(sha1_hex); -use File::Basename qw(dirname); +use File::Basename qw(basename dirname); use File::Copy qw(move); use File::Path qw(make_path); use File::Temp qw(tempfile); @@ -15,6 +15,7 @@ use POSIX qw(_exit); use lib "$RealBin/../tokenize"; use CregitLanguages; +use CregitSrcMl; binmode STDIN; binmode STDOUT; @@ -27,6 +28,7 @@ die "Tokenize command not defined. Use BFG_TOKENIZE_CMD\n" if not @tokenizeCommand; my $buildDir = "$RealBin/build"; make_path($buildDir); +my $inProcess = in_process_tokenizer(@tokenizeCommand); print "READY\n"; while (defined(my $header = )) { @@ -69,13 +71,33 @@ sub tokenize_blob { my $workDir = File::Temp->newdir("tokdir-XXXXX", DIR => $buildDir); write_file("$workDir/input.$ext", $contents); - my ($exit, $out, $err) = run_tokenizer("$workDir", $timeoutSecs, @tokenizeCommand, - "--language=$language", "input.$ext"); + my ($exit, $out, $err) = tokenize_file($language, "input.$ext", "$workDir", $timeoutSecs); return ($exit, "", $err) if $exit != 0; write_memo($memoFile, $out); return (0, $out, $err); } +sub tokenize_file { + my ($language, $inputName, $workDir, $timeoutSecs) = @_; + return $inProcess->tokenize($language, $inputName, $workDir, $timeoutSecs) + if $inProcess and $CregitLanguages::LANG_PARSER_REL{$language} eq $CregitLanguages::SRCML_PARSER_REL; + return run_tokenizer($workDir, $timeoutSecs, @tokenizeCommand, "--language=$language", $inputName); +} + +# Only a tokenize.pl command with the options that it gives to tokenizeSrcMl.pl +# can run in this process. Any other command starts once per blob. +sub in_process_tokenizer { + my ($program, @options) = @_; + return undef if basename($program) ne "tokenize.pl"; + my %config = (srcml2token => dirname($program) . "/srcMLtoken/srcml2token"); + for (@options) { + if (/\A--(srcml2token|ctags)=(.+)\z/) { $config{$1} = $2 } + elsif ($_ eq "--position") { $config{position} = 1 } + else { return undef } + } + return CregitSrcMl->new(%config); +} + sub run_tokenizer { my ($workDir, $timeoutSecs, @command) = @_; my $pid = fork() // die "cannot fork the tokenizer: $!\n";