-
Notifications
You must be signed in to change notification settings - Fork 56
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Use recyclable memory streams #280
Conversation
Awesome work as always @kerams ❤️ Before I publish this, I was actually checking out the implementation in Giraffe and it seems that Giraffe now registers a default RMS. Is it an idea for Remoting to actually try grab that registered RMS and use internally by default? i.e. use what's available:
Also, what do you think about
|
There's one problem.
I don't quite know what you want to achieve with that :).
Sure. |
I want to make it easy to require the already injected RMS from Giraffe: |> Remoting.withStreamManager (fun (context: HttpContext) ->
let rms = context.GetService<RecyclableMemoryStreamManager>()
Some rms
) Without having to define one on your own. Does this make sense? Should a default implementation already do that so that users of Giraffe & Remoting get a shared RMS without doing extra work? |
That would be unusable in Suave as it does not have any built-in DI as far as I know. We could have both your function and mine to configure the RMSM, but it would be a bit silly to complicate things so much for something almost no one will really bother to customize. I think what we have right now is the best solution since it's webserver/DI-agnostic. No injection through any containers, just direct plugging in via |
…leMemoryStream option (#280)
Then I'll leave it as is, I trust your judgement 💯 published a new batch of packages that includes the feature |
Just some more perf optimizations by reusing temporary output streams (we avoid reallocating an output buffer for every response) thanks to Microsoft.IO.RecyclableMemoryStream. Giraffe already uses this internally, so for the majority of users this won't be a new dependency.
I've also added
Remoting.withRecyclableMemoryStreamManager
This aligns with giraffe-fsharp/Giraffe#494, so that you could use recyclable memory streams on your own, in Giraffe and in Remoting, and all 3 would share the same memory stream pool (which has a couple of settings you can tune) if you wish.