Skip to content

IL: map the short-lived metadata-only PE reader - #20489

Open
auduchinok wants to merge 4 commits into
dotnet:mainfrom
auduchinok:ilread-mapEagerPEReader
Open

IL: map the short-lived metadata-only PE reader#20489
auduchinok wants to merge 4 commits into
dotnet:mainfrom
auduchinok:ilread-mapEagerPEReader

Conversation

@auduchinok

Copy link
Copy Markdown
Member

OpenILModuleReader's metadata-only branch — the FCS/IDE path, reduceMemoryUsage = Yes,
metadataOnly = Yes — opens a temporary PE reader to parse the headers, locate the metadata directory and
copy the resources out, then throws it away:

Allocation

project alloc base delta
FSharp.Common 2912.5 MB −306.6 MB (−10.5%)
Fantomas.Benchmarks 250.4 −38.5 (−15.4%)
consoleapp 142.6 −17.2 (−12.1%)
Oxpecker 491.2 −23.3 (−4.7%)
IcedTasks 678.5 −15.1 (−2.2%)
Fantomas.Core.Tests 1248.6 −26.8 (−2.1%)
FsToolkit.ErrorHandling 805.4 −13.9 (−1.7%)
Fantomas.Core 1152.2 −14.4 (−1.2%)
Prime 1202.3 −6.7 (−0.6%)
FSharp.Compiler.Service 19330.7 −7.1 (−0.0%)

Time

Warm whole-solution, 10 in-process passes

solution base ms delta
resharper 13670 −338 (−2.5%)
fantomas 3904 −64 (−1.7%)
oxpecker 2252 −16 (−0.7%)
icedtasks 6041 −18 (−0.3%)
fcsrepo 16098 +14 (+0.1%)
fstoolkit 3560 +6 (+0.2%)
prime 2127 +9 (+0.4%)
consoleapp 161 +2 (+1.2%)

This view exists only to parse the PE headers and copy the resources out, and is
disposed immediately after, so reading the whole assembly into a managed byte
array materialises megabytes to touch a few pages. Mapping it is safe because
neither pectxtEager nor pevEager may be captured by the results.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@github-actions

github-actions Bot commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

❗ Release notes required

You can open this PR in browser to add release notes: open in github.dev


✅ Found changes and release notes in following paths:

Change path Release notes path Description
`src/Compiler` docs/release-notes/.FSharp.Compiler.Service/11.0.100.md

@auduchinok auduchinok added the NO_RELEASE_NOTES Label for pull requests which signals, that user opted-out of providing release notes label Sep 8, 2026
@github-actions github-actions Bot added the AI-Tooling-Check-Scanned-Clean Tooling check: diff analyzed, no interesting infrastructure files label Sep 8, 2026
The cleanup lived in an explicit IDisposable implementation, which Stream.Dispose()
never reaches, so getBinaryFile's disposer left the file mapped until the safe handles
were finalized.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Comment thread src/Compiler/AbstractIL/ilread.fs
@T-Gro T-Gro added the AI-reviewed PR reviewed by AI review council label Sep 9, 2026
@T-Gro
T-Gro self-requested a review September 9, 2026 08:58
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@github-actions github-actions Bot added ⚠️ Affects-Compiler-Output Tooling check: PR touches IL emission or codegen ⚠️ Affects-Bootstrap Tooling check: PR touches compiler bootstrap chain labels Sep 9, 2026
@github-actions

github-actions Bot commented Sep 9, 2026

Copy link
Copy Markdown
Contributor

🔍 Tooling Safety Check — Affects-Bootstrap, Affects-Compiler-Output
Affects-Bootstrap: Changes compiler metadata-reading code used during bootstrap.
Affects-Compiler-Output: Changes compiler metadata and resource handling.

Generated by PR Tooling Safety Check · gpt56 595.2K ·

@auduchinok auduchinok removed the NO_RELEASE_NOTES Label for pull requests which signals, that user opted-out of providing release notes label Sep 10, 2026
@auduchinok
auduchinok force-pushed the ilread-mapEagerPEReader branch from 4ac1987 to 7cfe546 Compare September 10, 2026 09:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

⚠️ Affects-Bootstrap Tooling check: PR touches compiler bootstrap chain ⚠️ Affects-Compiler-Output Tooling check: PR touches IL emission or codegen AI-reviewed PR reviewed by AI review council AI-Tooling-Check-Scanned-Clean Tooling check: diff analyzed, no interesting infrastructure files

Projects

Status: New

Development

Successfully merging this pull request may close these issues.

2 participants