Skip to content
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

Should requests be mutable or immutable? #21

Closed
aantron opened this issue Apr 8, 2021 · 3 comments
Closed

Should requests be mutable or immutable? #21

aantron opened this issue Apr 8, 2021 · 3 comments
Milestone

Comments

@aantron
Copy link
Owner

aantron commented Apr 8, 2021

Request bodies are already mutable, because of the use of one-time callbacks/promises (rather than buffering streams). There are hidden mutable fields in requests for internal purposes, and there might be future hidden mutable caches.

Making requests fully mutable would simplify some of Dream's code, and save on allocations during immutable updates (but this can be mitigated in some other ways, as well).

I'm having a hard time seeing the use case for immutable requests — i.e. I don't think a Web app really ever needs to share them along different "execution paths."

Dream hasn't committed either way yet. It just provides an immutable-looking request API with a partially-mutable implementation.

@shawn-mcginty
Copy link
Contributor

For what it's worth, mutable requests sounds like the most sane option to me. I'm not certain but I would guess that it would improve performance and as you said above the API could look/feel immutable for dev happiness and to conform with idiomatic OCaml.

Maybe some testing could be done to see impact on performance?

@aantron
Copy link
Owner Author

aantron commented May 25, 2021

Maybe some testing could be done to see impact on performance?

I'll probably write some benchmark that passes requests through a biggish stack of middlewares that tweak them, and compare mutable vs. immutable semantics.

Mutable also sounds more sane to me, but it just doesn't "feel" functional :) However, this is OCaml :)

I'll probably ask on the forum at some point, while Dream is still in alpha releases, too.

@aantron
Copy link
Owner Author

aantron commented Dec 16, 2021

d7e81a2 makes requests and responses (messages) mutable. I decided not to profile it — after factoring out the core more carefully in preparation for the Dream client library, the server code written over the core practically required mutable messages in order to be sensible (to avoid returning requests and responses and rebinding them everywhere as the code gets split up into core and server modules).

The code does seem less silly without constant rebinding of messages everywhere. Messages were already mutable internally, because they had ephemeral streams, mutable multipart upload state, and I always planned the possibility of mutable caches for things like cookie parsing results. The above commit just "commits" to mutable messages fully, and takes advantage of some simplifications that are possible that way.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants