Prune the environment when installing an app #2833
Merged
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Hi team 👋
Context
There is a bug that's been around for some time (probably since the 0.x days) that I never paid too much attention to before because I didn't understand much about ZIO.
Currently, when we bootstrap / wire an app, we provide the current
ZEnvironment
in the NettyRuntime. This environment, however, contains pretty much all the services that were used to wire the app. Let's take the following app as an example:In this case, sending a request to the server will print the following in the console. As you can see, the environment contains 6 services in this case, where in theory it should only be containing our
Service
.Why is this a problem?
The way that
ZEnvironment
works, is that lookups areO(n)
complex (n = size of environment) when a service is accessed for the first time it goes into a private cache and then subsequent access for that service isO(1)
complex. Since the environment is immutable, anything that adds something to it, e.g.,ZIO.scoped
creates a new instance to it, so accessing that service becomes O(n) again. So having an environment as lean as possible will yield better performanceMain change
The changes in this PR ensure that the environment is pruned to contained only the services that are required to run the app. The downside to this is that we require an implicit
EnvironmentTag
now in theinstall
andserve
methods. I think that in most cases this should be okay, since most users would be usinginstall
orserve
when R is statically known.Alternative approach (no breaking changes)
An alternative that doesn't involve breaking changes would be to revert the implicit
EnvironmentTag
changes, and keep only the change made toAppRef
. This way, we leave it up to the user to properly clean the environment prior to callingserve
orinstall
. The downside to this is that it's unlikely that users will do this