Skip to content

Commit 1c5f5f5

Browse files
committed
remove some 'alloc' usage for a ~5% perf improvement
1 parent 570c9d9 commit 1c5f5f5

5 files changed

Lines changed: 168 additions & 100 deletions

File tree

.cargo/config.toml

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,3 +8,6 @@ rustflags = [
88
"-Ctarget-cpu=native",
99
"-Cforce-frame-pointers=yes",
1010
]
11+
12+
[target.x86_64-unknown-linux-gnu]
13+
rustflags = ["-Ctarget-cpu=native", "-Cforce-frame-pointers=yes"]

.gitignore

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -133,3 +133,5 @@ npm/
133133
# local
134134
**local-notes.md
135135
.cursor
136+
137+
profile.json.gz

Cargo.toml

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -63,5 +63,12 @@ resolver = "2"
6363

6464
[profile.release]
6565
codegen-units = 1
66-
lto = true
66+
lto = "fat"
67+
panic = "abort" # we don't care about panic machinery
6768
strip = "symbols"
69+
70+
# Profiling profile: release optimizations + debug symbols for samply/perf
71+
[profile.profiling]
72+
debug = true
73+
inherits = "release"
74+
strip = "none"

crates/berry-core/src/parse.rs

Lines changed: 103 additions & 95 deletions
Original file line numberDiff line numberDiff line change
@@ -2,10 +2,10 @@ use nom::IResult;
22
use nom::{
33
Parser,
44
branch::alt,
5-
bytes::complete::{is_not, tag, take_until, take_while1},
5+
bytes::complete::{is_not, tag, take_until, take_while, take_while1},
66
character::complete::{char, newline, space0, space1},
77
combinator::{map, opt, recognize},
8-
multi::{fold_many0, many0},
8+
multi::fold_many0,
99
sequence::{delimited, preceded, terminated},
1010
};
1111

@@ -45,18 +45,18 @@ pub fn parse_lockfile(file_contents: &str) -> IResult<&str, Lockfile> {
4545
};
4646

4747
// Parse all package entries as full Entries
48-
let (rest, entries) = many0(parse_entry).parse(rest)?;
48+
// Use fold_many0 to avoid intermediate Vec allocation from many0
49+
let (rest, entries) = fold_many0(parse_entry, Vec::new, |mut entries, entry| {
50+
entries.push(entry);
51+
entries
52+
})
53+
.parse(rest)?;
4954

5055
// Consume any trailing content (backticks, semicolons, whitespace, etc.)
51-
let (rest, _) = many0(alt((
52-
tag("`"),
53-
tag(";"),
54-
tag("\n"),
55-
tag(" "),
56-
tag("\t"),
57-
tag("\r"),
58-
)))
59-
.parse(rest)?;
56+
// perf: use take_while instead of many0 - zero allocation
57+
let (rest, _) =
58+
take_while(|c: char| c == '`' || c == ';' || c == '\n' || c == ' ' || c == '\t' || c == '\r')
59+
.parse(rest)?;
6060

6161
Ok((
6262
rest,
@@ -109,14 +109,14 @@ pub fn parse_descriptor_line(input: &str) -> IResult<&str, Vec<Descriptor>> {
109109
terminated(char('"'), preceded(newline, char(':'))),
110110
),
111111
delimited(char('"'), take_until("\":"), tag("\":")), // Quoted: "package@npm:version":
112-
terminated(take_until(":"), char(':')), // Unquoted: package@latest:
112+
terminated(take_until(":"), char(':')), // Unquoted: package@latest:
113113
))
114114
.parse(rest)?
115115
} else {
116116
// For normal descriptors, skip the newline-wrapped check entirely (performance optimisation)
117117
alt((
118118
delimited(char('"'), take_until("\":"), tag("\":")), // Quoted: "package@npm:version":
119-
terminated(take_until(":"), char(':')), // Unquoted: package@latest:
119+
terminated(take_until(":"), char(':')), // Unquoted: package@latest:
120120
))
121121
.parse(rest)?
122122
};
@@ -270,95 +270,103 @@ fn parse_patch_range(input: &str) -> IResult<&str, &str> {
270270
}
271271

272272
/// Parse indented key-value properties for a package
273+
/// perf: use `fold_many0` to build the Package directly without intermediate Vec allocation
273274
pub fn parse_package_properties(input: &str) -> IResult<&str, Package> {
274-
let (rest, properties) = many0(parse_property_line).parse(input)?;
275-
276-
// Consume any trailing whitespace and blank lines
277-
let (rest, _) = many0(alt((tag("\n"), tag(" "), tag("\t"), tag("\r")))).parse(rest)?;
278-
279-
// Build the package from the parsed properties
280-
let mut package = Package::new("unknown".to_string(), LinkType::Hard);
281-
282-
for property_value in properties {
283-
match property_value {
284-
PropertyValue::Simple(key, value) => match key {
285-
"version" => {
286-
package.version = Some(value.trim_matches('"').to_string());
287-
}
288-
"resolution" => {
289-
let raw = value.trim_matches('"').to_string();
290-
// Best-effort parse of the resolution into a Locator: split on '@' first occurrence
291-
// Examples: "debug@npm:1.0.0", "a@workspace:packages/a"
292-
if let Some(at_index) = raw.find('@') {
293-
let (name_part, reference) = raw.split_at(at_index);
294-
let ident = parse_name_to_ident(name_part);
295-
// split_at keeps the '@' on the right; remove it
296-
let reference = reference.trim_start_matches('@').to_string();
297-
package.resolution_locator = Some(Locator::new(ident, reference));
275+
// Build package directly using fold_many0 - no intermediate Vec allocation
276+
let (rest, package) = fold_many0(
277+
parse_property_line,
278+
|| Package::new("unknown".to_string(), LinkType::Hard),
279+
|mut package, property_value| {
280+
match property_value {
281+
PropertyValue::Simple(key, value) => match key {
282+
"version" => {
283+
package.version = Some(value.trim_matches('"').to_string());
284+
}
285+
"resolution" => {
286+
let raw = value.trim_matches('"').to_string();
287+
// Best-effort parse of the resolution into a Locator: split on '@' first occurrence
288+
// Examples: "debug@npm:1.0.0", "a@workspace:packages/a"
289+
if let Some(at_index) = raw.find('@') {
290+
let (name_part, reference) = raw.split_at(at_index);
291+
let ident = parse_name_to_ident(name_part);
292+
// split_at keeps the '@' on the right; remove it
293+
let reference = reference.trim_start_matches('@').to_string();
294+
package.resolution_locator = Some(Locator::new(ident, reference));
295+
}
296+
package.resolution = Some(raw);
297+
}
298+
"languageName" => {
299+
package.language_name = crate::package::LanguageName::new(value.to_string());
300+
}
301+
"linkType" => {
302+
package.link_type =
303+
LinkType::try_from(value).unwrap_or_else(|()| panic!("Invalid link type: {value}"));
304+
}
305+
"checksum" => {
306+
package.checksum = Some(value.to_string());
307+
}
308+
"conditions" => {
309+
package.conditions = Some(value.to_string());
310+
}
311+
_ => {
312+
panic!("Unknown property encountered in package entry: {key}");
313+
}
314+
},
315+
PropertyValue::Dependencies(dependencies) => {
316+
// Store the parsed dependencies in the package
317+
for (dep_name, dep_range) in dependencies {
318+
let ident = parse_dependency_name_to_ident(dep_name);
319+
let descriptor = Descriptor::new(ident, dep_range.to_string());
320+
package
321+
.dependencies
322+
.insert(descriptor.ident().clone(), descriptor);
298323
}
299-
package.resolution = Some(raw);
300-
}
301-
"languageName" => {
302-
package.language_name = crate::package::LanguageName::new(value.to_string());
303-
}
304-
"linkType" => {
305-
package.link_type =
306-
LinkType::try_from(value).unwrap_or_else(|()| panic!("Invalid link type: {value}"));
307-
}
308-
"checksum" => {
309-
package.checksum = Some(value.to_string());
310-
}
311-
"conditions" => {
312-
package.conditions = Some(value.to_string());
313-
}
314-
_ => {
315-
panic!("Unknown property encountered in package entry: {key}");
316-
}
317-
},
318-
PropertyValue::Dependencies(dependencies) => {
319-
// Store the parsed dependencies in the package
320-
for (dep_name, dep_range) in dependencies {
321-
let ident = parse_dependency_name_to_ident(dep_name);
322-
let descriptor = Descriptor::new(ident, dep_range.to_string());
323-
package
324-
.dependencies
325-
.insert(descriptor.ident().clone(), descriptor);
326324
}
327-
}
328-
PropertyValue::PeerDependencies(peer_dependencies) => {
329-
// Store the parsed peer dependencies in the package
330-
for (dep_name, dep_range) in peer_dependencies {
331-
let ident = parse_dependency_name_to_ident(dep_name);
332-
let descriptor = Descriptor::new(ident, dep_range.to_string());
333-
package
334-
.peer_dependencies
335-
.insert(descriptor.ident().clone(), descriptor);
325+
PropertyValue::PeerDependencies(peer_dependencies) => {
326+
// Store the parsed peer dependencies in the package
327+
for (dep_name, dep_range) in peer_dependencies {
328+
let ident = parse_dependency_name_to_ident(dep_name);
329+
let descriptor = Descriptor::new(ident, dep_range.to_string());
330+
package
331+
.peer_dependencies
332+
.insert(descriptor.ident().clone(), descriptor);
333+
}
336334
}
337-
}
338-
PropertyValue::Bin(binaries) => {
339-
// Store the parsed binary executables in the package
340-
for (bin_name, bin_path) in binaries {
341-
package
342-
.bin
343-
.insert(bin_name.to_string(), bin_path.to_string());
335+
PropertyValue::Bin(binaries) => {
336+
// Store the parsed binary executables in the package
337+
for (bin_name, bin_path) in binaries {
338+
package
339+
.bin
340+
.insert(bin_name.to_string(), bin_path.to_string());
341+
}
344342
}
345-
}
346-
PropertyValue::DependenciesMeta(meta) => {
347-
// Store the parsed dependency metadata in the package
348-
for (dep_name, dep_meta) in meta {
349-
let ident = parse_dependency_name_to_ident(dep_name);
350-
package.dependencies_meta.insert(ident, Some(dep_meta));
343+
PropertyValue::DependenciesMeta(meta) => {
344+
// Store the parsed dependency metadata in the package
345+
for (dep_name, dep_meta) in meta {
346+
let ident = parse_dependency_name_to_ident(dep_name);
347+
package.dependencies_meta.insert(ident, Some(dep_meta));
348+
}
351349
}
352-
}
353-
PropertyValue::PeerDependenciesMeta(meta) => {
354-
// Store the parsed peer dependency metadata in the package
355-
for (dep_name, dep_meta) in meta {
356-
let ident = parse_dependency_name_to_ident(dep_name);
357-
package.peer_dependencies_meta.insert(ident, dep_meta);
350+
PropertyValue::PeerDependenciesMeta(meta) => {
351+
// Store the parsed peer dependency metadata in the package
352+
for (dep_name, dep_meta) in meta {
353+
let ident = parse_dependency_name_to_ident(dep_name);
354+
package.peer_dependencies_meta.insert(ident, dep_meta);
355+
}
358356
}
359357
}
360-
}
361-
}
358+
package
359+
},
360+
)
361+
.parse(input)?;
362+
363+
// Consume any trailing whitespace and blank lines
364+
let (rest, _) = fold_many0(
365+
alt((tag("\n"), tag(" "), tag("\t"), tag("\r"))),
366+
|| (),
367+
|(), _| (),
368+
)
369+
.parse(rest)?;
362370

363371
Ok((rest, package))
364372
}

dev-docs/BENCHMARKING.md

Lines changed: 52 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -241,6 +241,7 @@ yarn4-patch.lock: 0 bytes heap usage
241241
- Use `fold_many0` instead of `many0`
242242
- Defer allocation until final data structures
243243
- Parse everything in one pass
244+
- avoid the `alloc` feature of nom
244245

245246
### Adding New Fixtures
246247

@@ -274,17 +275,64 @@ diff baseline-results.json current-results.json
274275
# Debug parsing issues
275276
RUST_LOG=debug cargo test
276277

277-
# Profile with flamegraph
278-
cargo install flamegraph
279-
cargo flamegraph --bench parser_benchmarks
280-
281278
# Memory profiling
282279
cargo run --bin berry-bench-bin -- -f large-fixture.lock -v
283280

284281
# Detailed Criterion analysis
285282
cargo bench --package berry-bench --bench parser_benchmarks -- --verbose
286283
```
287284

285+
## Profiling
286+
287+
Use `perf` or `samply` to generate flamegraphs and find optimization opportunities.
288+
289+
The project is configured with:
290+
291+
- Frame pointers enabled (`.cargo/config.toml`)
292+
- A dedicated `profiling` profile with debug symbols (`Cargo.toml`)
293+
294+
Use `--profile profiling` instead of `--release` to get readable symbols.
295+
296+
### Using samply (Recommended)
297+
298+
samply produces interactive flamegraphs in Firefox Profiler:
299+
300+
```bash
301+
# Build with profiling profile (release + debug symbols)
302+
cargo build --profile profiling --bin berry-bench-bin
303+
304+
# Profile in browser
305+
samply record ./target/profiling/berry-bench-bin -f resolutions-patches.yarn.lock -r 100
306+
```
307+
308+
### Profiling Large Lockfiles
309+
310+
For profiling with large lockfiles (10MB+), place your lockfile in `fixtures/` and run:
311+
312+
```bash
313+
# Build with profiling profile
314+
cargo build --profile profiling --bin berry-bench-bin
315+
316+
# Profile with samply
317+
samply record ./target/profiling/berry-bench-bin -f your-large-file.lock -r 50
318+
```
319+
320+
### Using perf (Alternative)
321+
322+
TODO - show dropped CPU cycles from jgenset 1bi
323+
324+
```bash
325+
# Build with profiling profile (sort of the same as release but we use it for some extra cargo settings)
326+
cargo build --profile profiling --bin berry-bench-bin
327+
328+
# Record profile
329+
perf record -g --call-graph dwarf \
330+
./target/profiling/berry-bench-bin -f resolutions-patches.yarn.lock -r 100
331+
332+
# View report
333+
perf report
334+
```
335+
288336
## Related
289337

290338
- [Criterion Documentation](https://docs.rs/criterion/) - Statistical benchmarking framework

0 commit comments

Comments
 (0)