Skip to content

Conversation

@TimothyMakkison
Copy link
Contributor

@TimothyMakkison TimothyMakkison commented Jan 29, 2026

Use ConditionalWeakTable to cache GetAttributeObjectInitializer. ConditionalWeakTable uses Object.ReferenceEquals for equality and does not count as a reference to the AttributeData, therefore it won't keep the object or attached compilation alive after it becomes stale.

image
  • On a related note, I'm pretty sure InterfaceCache should be causing a memory leak by keeping most stale Compilation objects alive. I have no idea why this hasn't become an issue.

  • Note that the benchmarks aren't completely accurate for real world scenarios. While GetAttributeObjectInitializer is slow, I suspect the main issue is each method in TUnit.TestProject has at least 8 identical licensing attributes, making this a slightly misleading benchmark

Before

Method Mean Error StdDev Gen0 Gen1 Allocated
Compile 499.5 ms 19.50 ms 57.48 ms 18000.0000 4000.0000 165.61 MB

After

Method Mean Error StdDev Median Gen0 Gen1 Allocated
Compile 174.6 ms 6.29 ms 17.96 ms 166.6 ms 10000.0000 3000.0000 96.14 MB

Attribute bloat

image

@TimothyMakkison
Copy link
Contributor Author

TimothyMakkison commented Jan 29, 2026

Realised that because the compilation is always the same in the benchmark, AttributeData would always be the same, meaning that all calls were cached. I reran the benchmarks, clearing the cache each time and here is the result.

Still excellent, but I might make AttributeWriter an instance and pass it around with its own internal cache. This way we don't have to bother with ConditionalWeakTable and can just use Dictionary.

Method Mean Error StdDev Median Gen0 Gen1 Allocated
Compile 225.6 ms 10.50 ms 29.45 ms 210.0 ms 11000.0000 4000.0000 104.79 MB

@TimothyMakkison
Copy link
Contributor Author

TimothyMakkison commented Jan 29, 2026

Sorry, rowing right now. I had a better idea to handle caching while respecting stateless generator runs. This shouldn't be merged

@TimothyMakkison TimothyMakkison marked this pull request as ready for review January 29, 2026 22:47
@TimothyMakkison
Copy link
Contributor Author

TimothyMakkison commented Jan 29, 2026

Instead of having a static ConditionalWeakTable, instead we make AttributeWriter an instance. This shares information between the GenerateTestMethodSource calls for a given compilation. This way the benchmarks remain accurate, we don't have to mess with weak references and we can expand this for more types. Replace the static dictionaries in InterfaceCache with this technique.

I did this previously with Mapperly, where CompilationContext contains all the shared type information for a given generator run.

@thomhurst thomhurst enabled auto-merge (squash) January 30, 2026 00:45
@thomhurst thomhurst merged commit c553097 into thomhurst:main Jan 30, 2026
8 of 10 checks passed
@TimothyMakkison
Copy link
Contributor Author

Is modular pipelines failing on windows just a quirk?

This was referenced Feb 2, 2026
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