fluttercommunity / get_it

Get It - Simple direct Service Locator that allows to decouple the interface from a concrete implementation and to access the concrete implementation from everywhere in your App. Maintainer: @escamoteur
https://pub.dev/packages/get_it
MIT License
1.36k stars 149 forks source link

Async factorties should have a `dependsOn` #340

Open feinstein opened 1 year ago

feinstein commented 1 year ago

I am dealing with a situation where I have an async singleton and a factory depends on it, so the factories should have a dependsOn for tracking dependencies.

escamoteur commented 1 year ago

why not awaiting the async singleon with isReady() or allReady() maybe you better explain what you try to achieve instead of opening one issue after the other asking for changes? You are the absolutely first of 1000s of users asking for things in this direction.

escamoteur commented 1 year ago

btw replying to my replies in the other issues would be nice

feinstein commented 1 year ago

I just saw your replies right now and I am replying them. I am opening different issues because I think those are different tasks to be completed.

Many people might not have asked for this because they don't care about memory footprint, or they are doing external async initialization and then injecting the ready objects into get_it (as I have seen many people doing). I am currently struggling to make get_it work with async initialization and when I raised this with a college he told me it doesn't work and I shouldn't use it. So, instead of doing like him and just abandoning the async functionality, I rather come here and try to contribute.

escamoteur commented 1 year ago

Ok, understood, but memory footprint because of objects registered inside get_it should never be a problem And async initialization works, only that lazy async singletons and factories are not supported because normally you initialize most of them at startup. A a sync lazy singletons is kind of a pretty weird Biest and I never used them myself. Making factories dependent on other singletons would be possible but typically you use a future builder at app startup to wait that step singletonss are completely initialized using allReady.

If you want to control multiple objects, scopes are the way to go. Am 17. Aug. 2023, 16:26 +0200 schrieb Michel Feinstein @.***>:

I just saw your replies right now and I am replying them. Many people might not have asked for this because they don't care about memory footprint, or they are doing external async initialization and then injecting the ready objects into get_it (as I have seen many people doing). I am currently struggling to make get_it work with async initialization and when I raised this with a college he told me it doesn't work and I shouldn't use it. So, instead of doing like him and just abandoning the async functionality, I rather come here and try to contribute. — Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you commented.Message ID: @.***>

feinstein commented 1 year ago

That being said, here's my scenario:

I have a LocalStorageDataSource that depends on 2 classes: SecureStorage and SharedPreferences. SharedPrefernces needs async initialization. Then the LocalStorageDataSource itself needs async initialization (calling an init function).

Then I have a TaskRepository that needs LocalStorageDataSource on it's constructor. And I have TasksCompletedUseCase that needs the TaskRepository. And also I have a TasksCubit that needs the TasksCompletedUseCase. The TasksCompletedUseCase is also needed by other Cubits.

I have been trying to tie all of these together using as less memory as possible and relying on dependesOn, but honestly I am 3 hours already into this all possible combinations of async factories, getAsync only end up with Exceptions. At the end it just appears that LocalStorageDataSource is not being called at all. I am not using allReady() at all, is it mandatory?

feinstein commented 1 year ago

Ok, understood, but memory footprint because of objects registered inside get_it should never be a problem

Why? If we are registering Singletons, by definition those are long lived objects, we might have cache there that grows indefinitely. Anything that's long lived cannot be assured to not leak memory, that's one of the main reasons why "turn it off and on again" fixes things, we reset long lived state, hence memory.

And async initialization works, only that lazy async singletons and factories are not supported because normally you initialize most of them at startup. A a sync lazy singletons is kind of a pretty weird Biest and I never used them myself.

We can initialize everything at startup, it works, but is it the best memory management possible? I don't think so. Objects should be created when necessary and only when necessary. This avoids bloated software, which is very important for developing countries, where people don't have access to phones with lot's of memory.

Making factories dependent on other singletons would be possible but typically you use a future builder at app startup to wait that step singletons are completely initialized using allReady.

Yeah, but isn't it more cleaner to just have get_it connect every single dependency by itself? I just declare what depends on what and magic happens when I ask for an instance. This way I don't have to lock app initialization for things that don't need to be initialized at startup, because they will only be used later on, if they will be used at all, since the user might not even select the page that's using it.

If you want to control multiple objects, scopes are the way to go.

Scopes work like navigation, but when we have a bottom navigation bar, where 5 pages co-exist at the same time, we have 5 different "branches" in our user-flow. This becomes very challenging to manage using scopes. Imagine I want to share the same Storage instance with just some of the sub-pages from those original 5 main pages, all of them having access to the same cache. Scopes won't help here, as I will destroy the singletons as I navigate back and fort into those sub-pages, across the bottom navigator.

escamoteur commented 1 year ago

I am currently on the road, get back to you Am 17. Aug. 2023, 16:46 +0200 schrieb Michel Feinstein @.***>:

Ok, understood, but memory footprint because of objects registered inside get_it should never be a problem Why? If we are registering Singletons, by definition those are long lived objects, we might have cache there that grows indefinitely. Anything that's long lived cannot be assured to not leak memory, that's one of the main reasons why "turn it off and on again" fixes things, we reset long lived state, hence memory. And async initialization works, only that lazy async singletons and factories are not supported because normally you initialize most of them at startup. A a sync lazy singletons is kind of a pretty weird Biest and I never used them myself. We can initialize everything at startup, it works, but is it the best memory management possible? I don't think so. Objects should be created when necessary and only when necessary. This avoids bloated software, which is very important for developing countries, where people don't have access to phones with lot's of memory. Making factories dependent on other singletons would be possible but typically you use a future builder at app startup to wait that step singletonss are completely initialized using allReady. If you want to control multiple objects, scopes are the way to go. Scopes work like navigation, but when we have a bottom navigation bar, where 5 pages co-exist at the same time, we have 5 different "branches" in our user-flow. This becomes very challenging to manage using scopes. Imagine I want to share the same Storage instance with just some of the sub-pages from those original 5 main pages, all of them having access to the same cache. Scopes won't help here, as I will destroy the singletons as I navigate back and fort into those sub-pages, across the bottom navigator. — Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you commented.Message ID: @.***>

feinstein commented 1 year ago

Ok, drive safe, I live in Australia, so I going to bed soon.

escamoteur commented 1 year ago

Why? If we are registering Singletons, by definition those are long lived objects, we might have cache there that grows indefinitely. Anything that's long lived cannot be assured to not leak memory, that's one of the main reasons why "turn it off and on again" fixes things, we reset long lived state, hence memory.

is not true for languages with a garbage collector. that is the case with classic c and c++ mainly.

then compared to the number of active classes in flutter our own app code is normally quite small and we seldom reach so many object that it gets a problem with memory. (and that from me who started with a C64) it took quite some time stop worrying about performance and memory on modern systems for the wrong reasons. Before you optimize for speed or memory first measure if you have a problem at all.

GetIt is a Servicelocator first and mainly. the async initiliastion is only meant to cooridnate the correct sequence while initializing and not to instanciate chains of dependent objects anytime you call a get.

If you need flexible handling of large objects you should design the management for that yourself and put the class that does it then into get_it.

IMHO Flutter developer only think in UI and packages and forget the power of OO business logic in between.

I recommend my talk from this year fluttercom in berlin: https://www.droidcon.com/2023/08/07/coding-the-happy-path-with-commands-and-exceptions/ to get some ideas, even if you don't want to use my packages that I use there.

also check out watch_it which has a pushScope method with an init function that will leave exactly as long as the widget where you call it.

escamoteur commented 1 year ago

is allReady mandatory

its not mandatory but by awaiting it you can be sure, that all async singletons are ready

feinstein commented 1 year ago

Why? If we are registering Singletons, by definition those are long lived objects, we might have cache there that grows indefinitely. Anything that's long lived cannot be assured to not leak memory, that's one of the main reasons why "turn it off and on again" fixes things, we reset long lived state, hence memory.

is not true for languages with a garbage collector. that is the case with classic c and c++ mainly.

then compared to the number of active classes in flutter our own app code is normally quite small and we seldom reach so many object that it gets a problem with memory. (and that from me who started with a C64) it took quite some time stop worrying about performance and memory on modern systems for the wrong reasons. Before you optimize for speed or memory first measure if you have a problem at all.

Sorry but I don't agree with this. Memory leaks also occur on garbage collected languages , and Singletons are a great source of those.

I also come from C, I used to develop embedded systems that had to run with less then 32kb of memory. Bear in mind that even though it was your personal experience with Flutter that it runs fine in the modern systems you used, Flutter also runs in embedded systems these days, like cars, TVs, RaspeberryPi and this is expanding.

So IMHO we need to let the developers choose how their apps are managed, and not make opinionated decisions on how most apps usually look like.

GetIt is a Servicelocator first and mainly. the async initiliastion is only meant to cooridnate the correct sequence while initializing and not to instanciate chains of dependent objects anytime you call a get.

If you need flexible handling of large objects you should design the management for that yourself and put the class that does it then into get_it.

I don't see how the "coordination of the correct sequence of initialization" differs from "instantiating chains of dependent objects when we call get". In my view they both walk together. I call get and the library looks for the factories, in order, to create the relevant objects. Some will be already in cache (Singletons), others will need to be recreated (Factories).

The factories can't be recreated without it's dependencies being created before, so if a factory depends on another factory, get_it needs to be aware of this and instantiate the correct objects, in the correct order, thus dealing with a dependency chain and the creation of objects.

IMHO Flutter developer only think in UI and packages and forget the power of OO business logic in between.

I don't see how this is related with our discussion, I am precisely architecturing the OO construction, unrealted to the UI, if I was thinking on UI I would have gone to Provider or riverpod for de dependency management.

escamoteur commented 1 year ago

OK, knowing now that you come from the embedded world, makes me at least understand why you are so caring about memory. Still you are doing premature optimization. Have you measured how much memory your own objects really take? It's true that Flutter is used on embedded systems but it needs at least a platform like a Raspberry Pie and it will never run on a Cortex M3.

Sure you can run out of memory in a garbage collected language but that is different from a memory leak because you did not dispose of memory but it is the result of programming mistakes e.g. creating an unlimited amount of objects. Especially Singleton that per definition exist only one of each should never make any problems in this area.

Measure how much memory your own singletons really take if you register them as long-lived objects. If it really turns out to be a problem let's talk again. But in general, get_it goal is not to optimize memory, but to make your objects easily accessible.

We are using in out current App which is pretty big with heavy with images, not one LazySingleton but mostly either long-lived singletons that then manage collections of share objects, or we push a scope for a certain route or widget and register some singletons in them that will live as long as the widget lives.

But indeed this discussion make me think about adding a new sort of scope that is not bound to the scope stack but can be handled completely separately from it. with the addition of the dropScope method we already made a first step in that direction as it allows to dispose of a scope independent of the stack position of it.

feinstein commented 1 year ago

Still you are doing premature optimization

That's not premature optimization. Premature optimization is considered bad practice because we spend time and make our code more complex, trying to solve a problem that might not even exist. But in this case we are not really spending any extra time, or making the code more complex.

Instead of writing:

getIt.registerSingleton<City>(City());

We write:

getIt.registerLazySingleton<City>(() => City());

The difference is literally 10 characters(Lazy and () =>). But in reality the difference is 0, since no one actually writes this, we just write once, and copy-paste each registration line, changing the class name.

So this way we can get performance for free, and that's always a good thing. This is not premature optimization, this is free performance, and this is actually how Flutter itself works, since we have build functions all over the place. This is also how Provider and Riverpod works, since both also define functions for creating dependencies. Functions are always more desirable as they can be invoked only when necessary.

Sure you can run out of memory in a garbage collected language but that is different from a memory leak

Nope, you get a memory leak on garbage collected languages every time a reference to an object is being hold without you realizing and the garbage collector won't remove that object from memory. For example, in Java an abstract class is always holding a reference from the class that declared that abstract class. So the containing class is leaking in memory as long as the abstract class is in memory.

You might think of a singleton as one simple object, but once this object holds references to others, that you have no control of, or come from another library, then you can't know how they will behave in memory, or if they will play nice.

One example is the groupBy operator from rxdart. It used to leak memory in the past, always accumulating data internally, but today we can control how it disposes it internal data. If your singleton used groupBy back then, it could be leaking memory every time a new data came from websockets for example.

escamoteur commented 1 year ago

I said it before, LazySingletons won't save you from memory consumption in case that you expect that they will get instantiated at some time during app execution. They save you processing time during startup.

It is premature optimization because you spend time and thoughts on a problem that hasn't happened in your app as I understand it. The change that a lazyAsyncSingleton would bring is pretty complex because other objects depending on it would never be ready if no one called that LazySingleton first.

To be sure that a factory has all needed async singletons ready you can always call isReady or isReadySync.

Am 20. Aug. 2023, 14:26 +0200 schrieb Michel Feinstein @.***>:

Still you are doing premature optimization That's not premature optimization. Premature optimization is considered bad practice because we spend time and make our code more complex, trying to solve a problem that might not even exist. But in this case we are not really spending any extra time, or making the code more complex. Instead of writing: getIt.registerSingleton(City()); We write: getIt.registerLazySingleton(() => City()); The difference is literally 10 characters(Lazy and () =>). But in reality the difference is 0, since no one actually writes this, we just write once, and copy-paste each registration line, changing the class name. So this way we can get performance for free, and that's always a good thing. This is not premature optimization, this is free performance, and this is actually how Flutter itself works, since we have build functions all over the place. This is also how Provider and Riverpod works, since both also define functions for creating dependencies. Functions are always more desirable as they can be invoked only when necessary. Sure you can run out of memory in a garbage collected language but that is different from a memory leak Nope, you get a memory leak on garbage collected languages every time a reference to an object is being hold without you realizing and the garbage collector won't remove that object from memory. For example, in Java an abstract class is always holding a reference from the class that declared that abstract class. So the containing class is leaking in memory as long as the abstract class is in memory. You might think of a singleton as one simple object, but once this object holds references to others, that you have no control of, or come from another library, then you can't know how they will behave in memory, or if they will play nice. One example is the groupBy operator from rxdart. It used to leak memory in the past, always accumulating data internally, but today we can control how it disposes it internal data. If your singleton used groupBy back then, it could be leaking memory every time a new data came from websockets for example. — Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you commented.Message ID: @.***>

feinstein commented 1 year ago

I said it before, LazySingletons won't save you from memory consumption in case that you expect that they will get instantiated at some time during app execution. They save you processing time during startup.

But what if that reference is never asked for during an app's execution? Won't it save memory?

escamoteur commented 1 year ago

How often would you expect that to happen, that exactly the money hungry objects are never called? I for such situations I would implement a manager singleton that lazyly creates objects if that is a really troubling situation. Or you register such objects only when a certain route of the app is triggered.  Registering a LazySingletons with a dependency might be possible so that you could use is Ready and all Ready on them. But it would take serious work and consideration to get it right and currently I focus my free time on watch_it and flutter_command. Am 20. Aug. 2023, 15:14 +0200 schrieb Michel Feinstein @.***>:

I said it before, LazySingletons won't save you from memory consumption in case that you expect that they will get instantiated at some time during app execution. They save you processing time during startup. But what if that reference is never asked for during an app's execution? Won't it save memory? — Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you commented.Message ID: @.***>

feinstein commented 1 year ago

I think the arguments about priority and time are valid, but they should not decide if a proposal is invalid.

That is, in order for a proposal to be invalid, it needs to lack merit, or be wrong. If it is valid it should be prioritised accordingly.

feinstein commented 1 year ago

FYI I found this injector package and all it's singletons are lazily created, which I think supports my case that I am not the only one in the world who cares about this.

escamoteur commented 1 year ago

The injector package is developed completely independently from me, they never even talked to my. Speculating why they made everything lazy doesn't prove anything. Am 1. Sept. 2023, 01:56 +0200 schrieb Michel Feinstein @.***>:

FYI I found this injector package and all it's singletons are lazily created, which I think supports my case that I am not the only one in the world who cares about this. — Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you commented.Message ID: @.***>

feinstein commented 1 year ago

I know they are not related to you, but I think it's a good and obvious speculation, but I can ask them why they did it, if you want.