Skip to content

Reduce allocations in XLSX query processing - #1002

Merged
michelebastione merged 3 commits into
v1.x-maintenancefrom
perf/reduce-query-allocations-clean
Sep 9, 2026
Merged

Reduce allocations in XLSX query processing#1002
michelebastione merged 3 commits into
v1.x-maintenancefrom
perf/reduce-query-allocations-clean

Conversation

@shps951023

@shps951023 shps951023 commented Sep 7, 2026

Copy link
Copy Markdown
Member

Summary

  • reduce short-lived allocations in XLSX query processing
  • construct ExcelRow directly with the required capacity instead of copying an intermediate dictionary
  • allocate formula metadata only for rows that contain formulas
  • use a value type for per-cell read results and parse cell references without allocating row substrings
  • add regression coverage for valid, invalid, and overflowing cell references
  • integrate sampled peak process memory into the existing BenchmarkDotNet pipeline through a custom IDiagnoser

Peak memory diagnostics

PeakMemoryDiagnoser samples the workload process every 20 ms during the actual workload phase of a separate BenchmarkDotNet diagnostic run. This avoids affecting the primary timing run and adds Peak Working Set and Peak Private Bytes to the normal Markdown, CSV, and HTML reports.

The diagnoser is enabled with BenchmarkPeakMemory=true. The Query benchmark workflow enables it automatically; Create and Template benchmarks keep their existing runtime.

A local run of MiniExcel Query produced:

Mean Allocated Peak Working Set Peak Private Bytes
6.234 s 8.06 GB 0.06 GB 0.04 GB

Peak values are sampled approximations and are machine-specific. Allocated remains cumulative managed allocation per operation.

Allocation improvement

Using the repository's 1,000,000-row x 10-column workbook on .NET 10, cumulative managed allocations decreased from 9,957.6 MB to approximately 8,223.5 MB, a reduction of about 1.73 GB (17.4%).

Validation

  • dotnet test tests/MiniExcelTests/MiniExcelTests.csproj -c Release --no-restore: 367 passed
  • dotnet build benchmarks/MiniExcel.Benchmarks/MiniExcel.Benchmarks.csproj -c Release -f net10.0 --no-restore: succeeded
  • full BenchmarkDotNet MiniExcel Query run completed with a separate diagnostic run and both peak-memory metrics
  • final MiniExcel QueryFirst integration run: 109.2 us mean, 64,072 KB peak working set, 36,684 KB peak private bytes, 55.23 KB allocated

@coderabbitai

coderabbitai Bot commented Sep 7, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Advanced

Run ID: ce1a6f95-1a44-44a1-bb0b-89a82ff41f81

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

Comment @coderabbitai help to get the list of available commands.

@shps951023

Copy link
Copy Markdown
Member Author

@michelebastione could you please review? thanks

@michelebastione

Copy link
Copy Markdown
Collaborator

Is this new MemoryValidation project a necessary addition? I don't see how that's better than simply running the main benchmarks using BenchmarkDotNet, besides the fact that we get an approximation for the peak memory usage. If that's what this is about I think we should try doing it by adding a custom MemoryDiagnoser to our BenchmarkDotNet pipeline before going for the "doing it raw" approach.

@shps951023

shps951023 commented Sep 8, 2026

Copy link
Copy Markdown
Member Author

Is this new MemoryValidation project a necessary addition? I don't see how that's better than simply running the main benchmarks using BenchmarkDotNet, besides the fact that we get an approximation for the peak memory usage. If that's what this is about I think we should try doing it by adding a custom MemoryDiagnoser to our BenchmarkDotNet pipeline before going for the "doing it raw" approach.

Yes, memory usage.
And I asked Benchmarkdotnet team, it can't support max memory usage data tracing MemoryDiagnoser 🙌
dotnet/BenchmarkDotNet#1740

But we can try it.

@shps951023

shps951023 commented Sep 8, 2026

Copy link
Copy Markdown
Member Author

AI suggestion :

I checked BenchmarkDotNet 0.15.0 in more detail and replaced the standalone MiniExcel.MemoryValidation project with a custom IDiagnoser in the existing benchmark pipeline.

Issue dotnet/BenchmarkDotNet#1740 explains why peak memory is not provided as a built-in metric and why it is machine-specific, but the extension API does expose the workload Process through DiagnoserActionParameters. The new PeakMemoryDiagnoser uses that API to sample working set and private bytes every 20 ms during BeforeActualRun/AfterActualRun.

It uses RunMode.ExtraRun, so sampling does not affect the primary timing run, and emits both values as normal BenchmarkDotNet metrics. It is opt-in through BenchmarkPeakMemory=true and enabled only for the Query workflow to avoid doubling the Create and Template benchmark durations.

I verified it with the full million-row Query benchmark. The generated BDN row reports 6.234 s mean, 8.06 GB allocated, 0.06 GB peak working set, and 0.04 GB peak private bytes. The standalone project has been removed in commit 35f461a. Thanks for pushing on this.

@michelebastione

Copy link
Copy Markdown
Collaborator

The PeakMemoryDiagnoser does not seem to be working always as intended, in some occasions the peak working set and peak private bytes are greater than the total memory allocated. I suggest removing it from this PR and working on it separately, what do you think?

@shps951023

Copy link
Copy Markdown
Member Author

The PeakMemoryDiagnoser does not seem to be working always as intended, in some occasions the peak working set and peak private bytes are greater than the total memory allocated. I suggest removing it from this PR and working on it separately, what do you think?

@michelebastione Ok, I will remove the benchmark part 🙌

@michelebastione

Copy link
Copy Markdown
Collaborator

I can take care of it

We'll work on it separately and add it to the codebase in a new PR
@michelebastione
michelebastione merged commit 521d848 into v1.x-maintenance Sep 9, 2026
4 checks passed
@shps951023

Copy link
Copy Markdown
Member Author

I can take care of it

Thanks ❤

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants