Improve performance

[sasa1977]

Problem

While I never bothered to measure the performance of the provider code, I'm aware of the two issues that might cause problems in some cases:

1. The values are never cached. Provider always goes to the source (currently only OS env) to fetch them again.
2. All the values are always fetched, cast, and validated, even when only one param is read (e.g. when invoking MyConfig.some_param()).

So far I didn't actually experience any problems with this in practice, primarily because I only had to deal with a relatively small number of params, and because these params were only read during the app startup (e.g. to configure endpoint or ecto repo).

However, using provider in a tight loop may significantly affect the code performance.

Proposed solution

1. Implement basic caching at the high-level API (generated modules). The client code using low-level API (e.g. Provider.fetch_all) is responsible for implementing its own caching scheme.
2. Replace the generated function validate!/0 with load!/0. The client code would typically invoke this function in app start callback. The function loads the data, casts and validates it (raising on error), and stores it to an ETS table (owned by the provider app). ETS table is proposed over persistent term to make the cache reload friendly. If the client code wants to refresh the config, they can simply invoke load!/0.
3. Fetching an individual param from the generated function boils down to an ETS lookup. Because params are fetched individually, it makes sense to use {module_name, param_name} as the key for each param.
4. Drop fetch_all from the generated code. This one is a bit tricky to implement with the proposed cache, and I currently can't see the use case for it. If people need it in practice, we can add it back.
5. Drop Provider.fetch_one and Provider.fetch_one! until someone requests this functionality.

With the proposed changes the lower-level API would be responsible for fetching the entire config, according to the given schema. The higher-level API (generated module) caches this data, and exposes strongly typed functions for fetching individual values from the cache.

[ankhers]

I am wondering, if the low-level API is split out into a behaviour so that we are allowed to implement whatever backend we want for configuration retrieval (ENV vars vs Hashicorp Vault vs AWS Secret manager vs ...), can this library provide an interface to a cache between all backends or would it be up to each individual backend to implement a cache? Would we want to provide a way for an entry to expire and automatically retrieve new values, or would the lib just retrieve new values when the application calls for configuration again?

[ankhers]

I believe I have a use case for this now. At work we are looking to start using AWS app config in order to work with feature flags. The PoC that we built in house basically mirrors this library, so I would just prefer to use this instead. The way that it worked is that it would reload the values from appconfig every n seconds and that data would be stored in an ETS table.

I think we could do something similar in this library. Have a process that gets started that would take configuration on how frequently we would want to reload values, this could be part of the use Provider call. I think we could use a single global value for the process and not per key. Maybe that could be added at a later point if the need arises.

I'm not entirely sure how we would want to do that exactly. We could inject a start_link function into the module that uses provider. But then we might have confusion around why sometimes we have to start a process and not other times.

[sasa1977]

You're proposing adding automatic cache refresh feature?

I'm not sure this is needed. If the original proposal is implemented, we'd able to do MyConfig.load!() which (re)caches the config. Doing this regularly could be as simple as :timer.apply_repeatedly(interval, MyConfig, :load!, []). Would that be enough for your use case?

If so, I suggest we first add the support for manual caching to the high-level interface (as proposed here). If you want to use provider in your project, we'd also need the adapter for AWS.

[ankhers]

You are right. Lets just get the cache implementation in here and we can figure out the implementation in our app.

Would you want to have the AppConfig adapter in this lib itself, or would you prefer I create a provider_appconfig lib that holds that specific integration?

[sasa1977]

I'd be fine with hosting adapters in this lib. We should only pay attention to mark any adapter deps as optional, and not to automatically start required processes in the provider app tree. So if some adapter requires some processes to be started, the client code would start those by adding a childspec like {SomeProvider, opts} somewhere in its own supervision tree. Does that make sense?

[ankhers]

It does.

I can see about getting started on this caching layer shortly.

[ankhers]

I think the proposed solution makes sense here. I believe we should be able to get started working through it.

[ankhers]

Something I came across when working on this. As things are currently implemented, you do not need to call the MyConfig.validate!/0 in order to use the values. If you simply call MyConfig.my_value/0, you will get a value (assuming it is set). Do you want to keep this behaviour? Or is this making it a hard requirement to first call MyConfig.load!/0?

[sasa1977]

IIRC this behaviour is probably an accidental consequence of the current implementation :-D I think it's fine to make people explicitly call load!, and otherwise raise if they try to fetch a value before the cache is primed. WDYT?

[ankhers]

I think that is fine as well. I just wanted to make sure you agreed with the direction before spending too much time on it.