Skip to content

Commit 9ff0e4f

Browse files
Speed up project-patch and purl checks (#1607) (#1701)
* Start fix for #1607 Assisted-by: Claude Code:claude-opus-5-5 * Speed up the project-patch check on big locks Every scan checks the project for its own patches (Yarn patch: locators, patch-package scripts) before touching a package, and hosted scans check again under the write lock. On a large Yarn Berry lock that check split every line of yarn.lock to find the few resolution lines that carry a patch: locator, and it recompiled the two patch-package regexes each time. Now it jumps straight to the "@patch:" occurrences and compiles the regexes once, only for a project whose scripts run patch-package. What it finds is unchanged. On the yarn-berry/rescan bench fixture this cuts about 20M instructions (~2%) from each scan. Refs #1607 Assisted-by: Claude Code:claude-opus-5-5 * Check canonical purls a byte at a time Scans key every crawled package's purl several times, and each key first checks whether the purl is already canonical. That check looked for '?', '#' and '%' by decoding every character. Scanning bytes gives the same answer (all three are ASCII, and no multi-byte UTF-8 character contains an ASCII byte) for a fraction of the cost: about 10M fewer instructions per yarn-berry/rescan bench run. Refs #1607 Assisted-by: Claude Code:claude-opus-5-5 * Guard the patch-line search against stalls The helper that finds yarn.lock lines holding a patch: locator now moves past each line it returns, so a needle that is empty or holds a newline can't keep matching the same spot. A test pins the documented line-ending rules instead of only comparing against str::lines. Refs #1607 Assisted-by: Claude Code:claude-opus-5-5 --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 99551b2 commit 9ff0e4f

2 files changed

Lines changed: 82 additions & 9 deletions

File tree

‎crates/socket-patch-core/src/patch/user_patches.rs‎

Lines changed: 76 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,9 @@
66
77
use std::collections::{BTreeSet, VecDeque};
88
use std::path::Path;
9+
use std::sync::LazyLock;
910

11+
use regex::Regex;
1012
use serde_json::Value;
1113

1214
use crate::formats::text::strip_bom;
@@ -107,8 +109,9 @@ impl UserPatchIndex {
107109
if let Some(text) = index.read(&yarn_lock, "npm").await {
108110
// Only Berry has patch: locators. Every Berry lock carries
109111
// builtin compat patches, so scan the resolution lines rather
110-
// than parse what can be a very large lockfile.
111-
for line in text.lines() {
112+
// than parse what can be a very large lockfile, and visit
113+
// only the few lines that can hold one.
114+
for line in lines_containing(&text, "@patch:") {
112115
let Some(value) = line.trim_start().strip_prefix("resolution:") else {
113116
continue;
114117
};
@@ -514,10 +517,12 @@ impl UserPatchIndex {
514517

515518
async fn patch_package(&mut self, dir: &Path, value: &Value) {
516519
self.patch_directory(&dir.join("patches"), false).await;
517-
let flag = regex::Regex::new(r#"--patch-dir(?:=|\s+)(?:"([^"]+)"|'([^']+)'|([^\s;&|]+))"#)
518-
.unwrap();
520+
// Compiled once, and only for a project whose scripts run patch-package.
521+
static FLAG: LazyLock<Regex> = LazyLock::new(|| {
522+
Regex::new(r#"--patch-dir(?:=|\s+)(?:"([^"]+)"|'([^']+)'|([^\s;&|]+))"#).unwrap()
523+
});
519524
// `cd` as a command, not the tail of a word like `abcd`.
520-
let cd = regex::Regex::new(r"(?:^|[\s;&|(])cd\s").unwrap();
525+
static CD: LazyLock<Regex> = LazyLock::new(|| Regex::new(r"(?:^|[\s;&|(])cd\s").unwrap());
521526
if let Some(scripts) = value["scripts"].as_object() {
522527
for script in scripts
523528
.values()
@@ -526,17 +531,17 @@ impl UserPatchIndex {
526531
{
527532
// A `cd` after patch-package (`patch-package && cd ios && pod
528533
// install`) can't move the directory it already read.
529-
let moved = cd
534+
let moved = CD
530535
.find(script)
531536
.is_some_and(|cd| cd.start() < script.rfind("patch-package").unwrap_or(0));
532-
if moved || (script.contains("--patch-dir") && !flag.is_match(script)) {
537+
if moved || (script.contains("--patch-dir") && !FLAG.is_match(script)) {
533538
self.unknown(
534539
"npm",
535540
&dir.join("package.json"),
536541
"cannot resolve patch-package directory without executing the script",
537542
);
538543
}
539-
for captures in flag.captures_iter(script) {
544+
for captures in FLAG.captures_iter(script) {
540545
let path = captures.iter().skip(1).flatten().next().unwrap().as_str();
541546
if path.contains(['$', '`']) || moved {
542547
self.unknown(
@@ -724,6 +729,27 @@ fn locator_target(locator: &str) -> (&str, Option<&str>) {
724729
}
725730
}
726731

732+
/// The lines of `text` that contain `needle`, each once, as `str::lines`
733+
/// spells them (a `\r` before a `\n` is dropped, a final bare one kept). Searching for the
734+
/// needle skips the rest of the text instead of splitting every line.
735+
/// `needle` must be non-empty and on one line, so each match moves on.
736+
fn lines_containing<'a>(text: &'a str, needle: &'a str) -> impl Iterator<Item = &'a str> + 'a {
737+
debug_assert!(!needle.is_empty() && !needle.contains('\n'));
738+
let mut from = 0;
739+
std::iter::from_fn(move || {
740+
let at = from + text.get(from..)?.find(needle)?;
741+
let start = text[..at].rfind('\n').map_or(0, |i| i + 1);
742+
let Some(end) = text[at..].find('\n').map(|i| at + i) else {
743+
// Past the end: the next call finds nothing.
744+
from = text.len() + 1;
745+
return Some(&text[start..]);
746+
};
747+
from = end + 1;
748+
let line = &text[start..end];
749+
Some(line.strip_suffix('\r').unwrap_or(line))
750+
})
751+
}
752+
727753
fn lock_options() -> serde_saphyr::Options {
728754
// Lockfiles of large monorepos exceed the default 250k-node budget, and a
729755
// parse failure would protect every npm package.
@@ -861,6 +887,48 @@ mod tests {
861887
assert!(index.conflicts("pkg:npm/@s/pkg@1.3.0").is_empty());
862888
}
863889

890+
#[test]
891+
fn lines_containing_matches_a_filtered_lines_walk() {
892+
for text in [
893+
"",
894+
"@patch:",
895+
"a\nb@patch:c\n",
896+
"x@patch:y@patch:z\r\nno\r\n q@patch:\r\n",
897+
"\n\n@patch:\n\n",
898+
"a@patch:\nb@patch:",
899+
"@patch:@patch:\r",
900+
"lone\rcarriage@patch:\r\rx\n",
901+
"\u{e9}@patch:\u{e9}\n\u{e9}",
902+
] {
903+
let slow: Vec<&str> = text.lines().filter(|l| l.contains("@patch:")).collect();
904+
let fast: Vec<&str> = lines_containing(text, "@patch:").collect();
905+
assert_eq!(fast, slow, "{text:?}");
906+
}
907+
// The documented contract, independent of the toolchain's `lines`.
908+
let got: Vec<&str> = lines_containing("a@patch:\r\nb@patch:\r", "@patch:").collect();
909+
assert_eq!(got, ["a@patch:", "b@patch:\r"]);
910+
}
911+
912+
#[tokio::test]
913+
async fn yarn_lock_patches_are_found_among_many_unpatched_entries() {
914+
let tmp = tempfile::tempdir().unwrap();
915+
let mut lock = String::from("__metadata:\r\n version: 8\r\n");
916+
for i in 0..200 {
917+
lock.push_str(&format!(
918+
"\"dep-{i}@npm:1.0.0\":\r\n version: 1.0.0\r\n resolution: \"dep-{i}@npm:1.0.0\"\r\n"
919+
));
920+
}
921+
lock.push_str("\"first@patch:first@npm%3A1.0.0#./a.patch\":\r\n resolution: \"first@patch:first@npm%3A1.0.0#./a.patch::version=1.0.0&hash=a\"\r\n");
922+
lock.push_str("\"ts@patch:ts@npm%3A5.0.0#optional!builtin<compat/typescript>\":\r\n resolution: \"ts@patch:ts@npm%3A5.0.0#optional!builtin<compat/typescript>::version=5.0.0&hash=b\"\r\n");
923+
lock.push_str("\"last@patch:last@npm%3A2.0.0#./b.patch\":\n resolution: \"last@patch:last@npm%3A2.0.0#./b.patch::version=2.0.0&hash=c\"");
924+
write(tmp.path(), "yarn.lock", &lock);
925+
let index = UserPatchIndex::discover(tmp.path()).await;
926+
assert!(!index.conflicts("pkg:npm/first@1.0.0").is_empty());
927+
assert!(!index.conflicts("pkg:npm/last@2.0.0").is_empty());
928+
assert!(index.conflicts("pkg:npm/ts@5.0.0").is_empty());
929+
assert!(index.conflicts("pkg:npm/dep-7@1.0.0").is_empty());
930+
}
931+
864932
#[tokio::test]
865933
async fn patch_package_supports_scoped_nested_sequenced_dev_and_custom_directories() {
866934
let tmp = tempfile::tempdir().unwrap();

‎crates/socket-patch-core/src/utils/purl_key.rs‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,9 @@ pub fn canonical_base_purl(purl: &str) -> String {
6161
/// no qualifiers, subpath or percent escapes, and a lowercase type whose
6262
/// names and versions keep their spelling.
6363
fn is_canonical_base(purl: &str) -> bool {
64-
if purl.trim().len() != purl.len() || purl.contains(['?', '#', '%']) {
64+
// A byte scan: these are ASCII, and no multi-byte UTF-8 sequence
65+
// contains an ASCII byte. Matching chars costs a decode per char.
66+
if purl.trim().len() != purl.len() || purl.bytes().any(|b| matches!(b, b'?' | b'#' | b'%')) {
6567
return false;
6668
}
6769
let Some((ty, _)) = purl
@@ -272,6 +274,9 @@ mod tests {
272274
"pkg:maven/org.Example/Lib@1.0",
273275
"pkg:gem/nokogiri@1.16.5",
274276
"pkg:cargo/serde@1.0.0",
277+
"pkg:npm/caf\u{e9}@1.0.0",
278+
"pkg:npm/caf\u{e9}%41@1.0.0",
279+
"pkg:npm/\u{2603}@1.0.0?x=\u{e9}",
275280
"pkg:npm/no-version",
276281
"pkg:npm/@scope",
277282
"pkg:npm",

0 commit comments

Comments
 (0)