search

openai / openai-dotnet

The official .NET library for the OpenAI API
https://www.nuget.org/packages/OpenAI/AbsoluteLatest
MIT License
722 stars 62 forks source link

Remove seemingly unnecessary string allocation from ClientUriBuilder.AppendPath #37

Closed stephentoub closed 3 weeks ago

stephentoub commented 3 weeks ago

There aren't any tests in the repo currently to help me validate this, but it looks like this PathBuilder.ToString() call in ClientUriBuilder.AppendPath is superfluous. Every use I see of ClientUriBuilder ends with a call to ToUri, which will itself call PathBuilder.ToString() and overwrite the UriBuilder's Path. Further, the corresponding ClientUriBuilder.AppendQuery doesn't similarly write to UriBuilder.Query.

stephentoub commented 3 weeks ago

this is a generated file. we need to fix this in the generator. @m-nash

I see now it's in a generated folder. What's it generated from?

trrwilson commented 3 weeks ago

this is a generated file. we need to fix this in the generator. @m-nash

I see now it's in a generated folder. What's it generated from?

I believe it's from this line in the code emitter project: https://github.com/Azure/autorest.csharp/blob/bd37a920d0a545e06ebb754b6c2fb90b565bc571/src/AutoRest.CSharp/Common/Output/Models/Types/HelperTypeProviders/System/ClientUriBuilderProvider.cs#L162

@m-nash should confirm.

The optimization will be picked up automatically once we regenerate with updated tooling.

stephentoub commented 3 weeks ago

Thanks. I'm curious why this is generated. Does the generated code end up differing from project to project?

stephentoub commented 3 weeks ago

Closing as this code is generated