Skip to content

Reinstate caching in schema generation - #5908

Merged
eiriktsarpalis merged 7 commits into
dotnet:mainfrom
eiriktsarpalis:aifunctionfactory-caching
Feb 19, 2025
Merged

Reinstate caching in schema generation#5908
eiriktsarpalis merged 7 commits into
dotnet:mainfrom
eiriktsarpalis:aifunctionfactory-caching

Conversation

@eiriktsarpalis

@eiriktsarpaliseiriktsarpalis commented Feb 15, 2025

Copy link
Copy Markdown
Member

I spent a bit of time investigating the performance of AIFunctionFactory.Create where I discovered that schema generation is the most expensive factor by far is schema derivation. Even though the schema generation routines used to employ a caching mechanism in the past, this got removed during the recent refactorings since it no longer is necessary for leaf clients to resolve schemas on the fly.

This PR reinstates a different version of the caching mechanism for the purpose of speeding up AIFunctionFactory.Create when applied multiple times on the same delegate.

Benchmarks

[MemoryDiagnoser]publicclassBenchmark{privatestaticreadonlyFunc<string,int,MyPoco?,MyPoco>_delegate=MyFunc;[Benchmark]publicJsonElementCreateFunctionSchema()=>AIJsonUtilities.CreateFunctionJsonSchema(_delegate.Method);[Benchmark]publicJsonElementCreateFunctionSchema_CustomDescription()=>AIJsonUtilities.CreateFunctionJsonSchema(_delegate.Method,description:"custom description");[Benchmark]publicJsonElementCreateFunctionSchema_NewOptions()=>AIJsonUtilities.CreateFunctionJsonSchema(_delegate.Method,serializerOptions:new(JsonSerializerOptions.Default));[Benchmark]publicAIFunctionCreateAIFunction()=>AIFunctionFactory.Create(_delegate);[Benchmark]publicAIFunctionCreateAIFunction_CustomDescription()=>AIFunctionFactory.Create(_delegate,description:"custom description");[Benchmark]publicAIFunctionCreateAIFunction_NewOptions()=>AIFunctionFactory.Create(_delegate,serializerOptions:new(JsonSerializerOptions.Default));[Description("My awesome function")]publicstaticMyPocoMyFunc([Description("Parameter 1")]stringparam1,intparam2=42,MyPoco?other=null)=>thrownewNotImplementedException();publicrecordMyPoco(intX,intY);}

Main

MethodMeanErrorStdDevGen0Allocated
CreateFunctionSchema6.345 us0.0848 us0.0708 us2.41099.95 KB
CreateFunctionSchema_CustomDescription6.041 us0.0357 us0.0298 us2.34999.7 KB
CreateFunctionSchema_NewOptions6.438 us0.0260 us0.0230 us2.19739.1 KB
CreateAIFunction6.706 us0.0344 us0.0322 us2.594010.65 KB
CreateAIFunction_CustomDescription6.298 us0.0393 us0.0368 us2.533010.45 KB
CreateAIFunction_CustomOptions6.683 us0.0300 us0.0281 us2.38049.85 KB

PR

MethodMeanErrorStdDevGen0Allocated
CreateFunctionSchema6,679.98 ns35.900 ns31.824 ns2.410910193 B
CreateFunctionSchema_CustomDescription6,126.56 ns43.308 ns38.391 ns2.34999929 B
CreateFunctionSchema_NewOptions6,494.00 ns59.036 ns52.334 ns2.19739318 B
CreateAIFunction20.20 ns0.097 ns0.081 ns--
CreateAIFunction_CustomDescription28.19 ns0.182 ns0.171 ns0.013456 B
CreateAIFunction_NewOptions7,309.19 ns72.739 ns64.481 ns2.563510854 B
Microsoft Reviewers: Open in CodeFlow

@dotnet-comment-bot

Copy link
Copy Markdown
Collaborator

‼️Found issues‼️

ProjectCoverage TypeExpectedActual
Microsoft.Extensions.AI.OpenAILine7767.92 🔻
Microsoft.Extensions.AI.OpenAIBranch7749.8 🔻
Microsoft.Extensions.Caching.HybridLine8682.77 🔻
Microsoft.Extensions.AI.OllamaLine8078.2 🔻
Microsoft.Extensions.AI.Evaluation.QualityLine887.57 🔻
Microsoft.Extensions.AI.Evaluation.QualityBranch8816.42 🔻
Microsoft.Extensions.AI.EvaluationLine8858.67 🔻
Microsoft.Extensions.AI.EvaluationBranch8856.67 🔻
Microsoft.Extensions.AI.Evaluation.ConsoleLine888.26 🔻
Microsoft.Extensions.AI.Evaluation.ConsoleBranch8817.07 🔻
Microsoft.Extensions.AI.AzureAIInferenceLine9190.09 🔻
Microsoft.Extensions.AI.AzureAIInferenceBranch9186.76 🔻
Microsoft.Gen.MetadataExtractorLine9857.35 🔻
Microsoft.Gen.MetadataExtractorBranch9862.5 🔻
Microsoft.Extensions.AI.Evaluation.ReportingLine8872.06 🔻
Microsoft.Extensions.AI.Evaluation.ReportingBranch8864.8 🔻

🎉 Good job! The coverage increased 🎉
Update MinCodeCoverage in the project files.

ProjectExpectedActual
Microsoft.Extensions.AI8889
Microsoft.Extensions.AI.Abstractions8384

Full code coverage report: https://dev.azure.com/dnceng-public/public/_build/results?buildId=953290&view=codecoverage-tab

@stephentoub

Copy link
Copy Markdown
Member

This PR reinstates a different version of the caching mechanism for the purpose of speeding up AIFunctionFactory.Create when applied multiple times on the same delegate.

We expect that to be something folks do, recreating an AIFunction for the same delegate over and over? I was expecting our guidance would be to create them once and cache them.

@stephentoub

Copy link
Copy Markdown
Member

Benchmark

What do the numbers before/after look like if you first add 512 methods to the cache and then benchmark calling Create with a different one? I'm wondering what the overhead of accessing the ConditionalWeakTable, accessing the ConcurrentDictionary, running the GetHashCode, etc. looks like when it doesn't pay dividends. Presumably the common case will be that across the whole process, typically the default JsonSerializerOptions will be used, so the 512 limit will end up applying to all use of AIFunctionFactory across the process.

@eiriktsarpalis

Copy link
Copy Markdown
MemberAuthor

We expect that to be something folks do, recreating an AIFunction for the same delegate over and over?

When working on the demo I felt enticed to incorporate the AIFunction declaration directly on the method like so:

chatOptions.Tools=[AIFunctionFactory.Create(GetNearestPorts)];

Which got me thinking that users would likely fall into the same performance trap.

I'm wondering what the overhead of accessing the ConditionalWeakTable, accessing the ConcurrentDictionary, running the GetHashCode, etc. looks like when it doesn't pay dividends.

That scenario should be represented by the benchmarks creating a new JSO every time. As you can see, it does introduce performance regression compared to what we have in main today.

@dotnet-comment-bot

Copy link
Copy Markdown
Collaborator

‼️Found issues‼️

ProjectCoverage TypeExpectedActual
Microsoft.Extensions.AI.OllamaLine8078.2 🔻
Microsoft.Extensions.AI.EvaluationLine8858.67 🔻
Microsoft.Extensions.AI.EvaluationBranch8856.67 🔻
Microsoft.Extensions.AI.Evaluation.QualityLine887.57 🔻
Microsoft.Extensions.AI.Evaluation.QualityBranch8816.42 🔻
Microsoft.Extensions.AI.AzureAIInferenceLine9190.09 🔻
Microsoft.Extensions.AI.AzureAIInferenceBranch9186.76 🔻
Microsoft.Extensions.AI.Evaluation.ReportingLine8872.06 🔻
Microsoft.Extensions.AI.Evaluation.ReportingBranch8864.8 🔻
Microsoft.Gen.MetadataExtractorLine9857.35 🔻
Microsoft.Gen.MetadataExtractorBranch9862.5 🔻
Microsoft.Extensions.AI.OpenAILine7767.92 🔻
Microsoft.Extensions.AI.OpenAIBranch7749.8 🔻
Microsoft.Extensions.AI.Evaluation.ConsoleLine888.26 🔻
Microsoft.Extensions.AI.Evaluation.ConsoleBranch8817.07 🔻

🎉 Good job! The coverage increased 🎉
Update MinCodeCoverage in the project files.

ProjectExpectedActual
Microsoft.Extensions.AI.Abstractions8384
Microsoft.Extensions.Caching.Hybrid8687
Microsoft.Extensions.AI8889

Full code coverage report: https://dev.azure.com/dnceng-public/public/_build/results?buildId=953615&view=codecoverage-tab

@stephentoub

Copy link
Copy Markdown
Member

When working on the demo I felt enticed to incorporate the AIFunction declaration directly on the method like so:

Should we cache the AI Functions instead then?

@eiriktsarpalis

Copy link
Copy Markdown
MemberAuthor

When working on the demo I felt enticed to incorporate the AIFunction declaration directly on the method like so:

Should we cache the AI Functions instead then?

I tried that and have a working prototype locally. What I did discover is that schema generation accounts for ~95% of the cost (as can be seen from the benchmark results) and caching on the AIFunctionMetadata level is trickier since there are more variables at play. It seemed like an obvious win to do it on the schema generation APIs instead.

@stephentoub

stephentoub commented Feb 15, 2025

Copy link
Copy Markdown
Member

Those numbers are very suspect to me, e.g.

CreateAIFunction | 56.31 ns | 0.404 ns | 0.378 ns | 0.0172 | 72 B

That's suggesting with this change AIFunctionFactory.Create(delegate) only allocates 72 bytes and only took 56 nanoseconds? Without caching the AIFunction? It's doing a non-trivial amount of reflection. The AIFunction object alone must be close to 72 bytes, plus the delegates that get created for marshaling.

So I pulled down your change, and instead I see numbers like this:

MethodMeanErrorStdDevAllocated
CreateAIFunction1.214 us0.0194 us0.0182 us816 B

Can you re-confirm what it is you're measuring?

@eiriktsarpalis

Copy link
Copy Markdown
MemberAuthor

Here's updated numbers from a new run:

MethodMeanErrorStdDevGen0Gen1Allocated
CreateFunctionSchema26.01 ns0.093 ns0.087 ns---
CreateFunctionSchema_CustomDescription38.41 ns0.172 ns0.161 ns---
CreateFunctionSchema_NewOptions7,505.62 ns139.908 ns130.870 ns2.3804-10167 B
CreateAIFunction612.79 ns5.975 ns5.589 ns0.1945-816 B
CreateAIFunction_CustomDescription224.36 ns1.081 ns0.903 ns0.1452-608 B
CreateAIFunction_NewOptions7,657.49 ns61.634 ns54.637 ns2.56350.061010779 B

And here's the same benchmarks with additional caching implemented on the AIFunction layer:

MethodMeanErrorStdDevGen0Allocated
CreateFunctionSchema27.15 ns0.085 ns0.079 ns--
CreateFunctionSchema_CustomDescription32.35 ns0.215 ns0.201 ns--
CreateFunctionSchema_NewOptions7,648.95 ns140.465 ns196.912 ns2.410910167 B
CreateAIFunction60.11 ns0.307 ns0.287 ns0.017272 B
CreateAIFunction_CustomDescription66.00 ns0.256 ns0.227 ns0.0305128 B
CreateAIFunction_NewOptions8,604.40 ns60.222 ns53.385 ns2.746611692 B

So the numbers I quoted originally were most likely recorded with those changes accidentally unstashed. I've updated the OP to reflect the correct numbers.

@eiriktsarpalis

Copy link
Copy Markdown
MemberAuthor

@stephentoub as discussed, I pushed updates using caching on the AIFunctionFactory layer only. Here are the updated benchmark numbers:

MethodMeanErrorStdDevGen0Allocated
CreateFunctionSchema6,679.98 ns35.900 ns31.824 ns2.410910193 B
CreateFunctionSchema_CustomDescription6,126.56 ns43.308 ns38.391 ns2.34999929 B
CreateFunctionSchema_NewOptions6,494.00 ns59.036 ns52.334 ns2.19739318 B
CreateAIFunction20.20 ns0.097 ns0.081 ns--
CreateAIFunction_CustomDescription28.19 ns0.182 ns0.171 ns0.013456 B
CreateAIFunction_NewOptions7,309.19 ns72.739 ns64.481 ns2.563510854 B

@dotnet-comment-bot

Copy link
Copy Markdown
Collaborator

🎉 Good job! The coverage increased 🎉
Update MinCodeCoverage in the project files.

ProjectExpectedActual
Microsoft.Extensions.Caching.Hybrid8287
Microsoft.Gen.MetadataExtractor5770

Full code coverage report: https://dev.azure.com/dnceng-public/public/_build/results?buildId=955543&view=codecoverage-tab

@dotnet-comment-bot

Copy link
Copy Markdown
Collaborator

🎉 Good job! The coverage increased 🎉
Update MinCodeCoverage in the project files.

ProjectExpectedActual
Microsoft.Extensions.Caching.Hybrid8287
Microsoft.Gen.MetadataExtractor5770

Full code coverage report: https://dev.azure.com/dnceng-public/public/_build/results?buildId=955619&view=codecoverage-tab

@eiriktsarpalis
eiriktsarpalis enabled auto-merge (squash) February 19, 2025 13:53
Comment threadsrc/Libraries/Microsoft.Extensions.AI/Functions/AIFunctionFactory.cs Outdated
Comment threadsrc/Libraries/Microsoft.Extensions.AI/Functions/AIFunctionFactory.cs Outdated

@stephentoubstephentoub left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice, thanks.

@dotnet-comment-bot

Copy link
Copy Markdown
Collaborator

‼️Found issues‼️

ProjectCoverage TypeExpectedActual
Microsoft.Extensions.AILine8988.7 🔻

🎉 Good job! The coverage increased 🎉
Update MinCodeCoverage in the project files.

ProjectExpectedActual
Microsoft.Gen.MetadataExtractor5770

Full code coverage report: https://dev.azure.com/dnceng-public/public/_build/results?buildId=956635&view=codecoverage-tab

@eiriktsarpalis
eiriktsarpalis merged commit 66eca03 into dotnet:mainFeb 19, 2025
@eiriktsarpalis
eiriktsarpalis deleted the aifunctionfactory-caching branch February 19, 2025 17:27
@jeffhandleyjeffhandley added the area-ai Microsoft.Extensions.AI libraries label Mar 7, 2025
@github-actionsgithub-actionsBot locked and limited conversation to collaborators Apr 6, 2025
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

area-aiMicrosoft.Extensions.AI libraries

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

@eiriktsarpalis@dotnet-comment-bot@stephentoub@jeffhandley