kalliope-project / kalliope

Kalliope is a framework that will help you to create your own personal assistant.
https://kalliope-project.github.io/
GNU General Public License v3.0
1.71k stars 230 forks source link

Added cache to Utils.get_dynamic_class_instantiation() #659

Closed juergenpabel closed 3 years ago

juergenpabel commented 3 years ago

This implements a caching mechanism to Utils.get_dynamic_class_instantiation() by storing a reference to the to-be-instantiad class in a directory in Utils.klass_cache with the package_path as the key. This should be a side-effect free change that optimizes runtime for class instantiations where those classes maintain state in class attributes.

My specific use case is the vosk STT plugin, which (now) stores its (~50MB) model in a class attribute in order to reduce class instantiation time for each STT pass (juergenpabel/kalliope-vosk@6d63709). Only the first instantation takes about 1-2 secs on a pi4, subsequent instantiations are now instantanious.

Sispheor commented 3 years ago

Really nice. Thanks. Would it be possible to add a unit test to validate the behavior?

juergenpabel commented 3 years ago

How about a test_get_dynamic_class_cached_instantiation in test_utils.py that essentially does what test_get_dynamic_classinstantiation() does but adds a second instantiation with in between the setting of a class attribute on Say and checks that this attribute exists in the __class_\ member of the second instantiation?

One could of course test whether the klass_cache member contains the entry, but that would be an (internal) implementation test - which probably shouldn't be done. Or: we add methods to Utils to query/reset the klass_cache and than unit test that behavious. But what would the real world use case for those methods be (aside from unit testing)?

Sispheor commented 3 years ago

It was to validate that the class is well instantiated once. And then loaded from the cache. Like we've tested the singleton. Just a small thing to prevent regression on what you've added here.

You can compare the 2 objects id and validate that it's the same instance.

juergenpabel commented 3 years ago

I'll get back to this within the next days, please hang on for a bit...

juergenpabel commented 3 years ago

Done. But the instances need to be different (not the same as suggested in the previous comment), as only the class itself is cache, "fresh" class instantiations are (still) returned by get_dynamic_class_instantiation().

Sispheor commented 3 years ago

All good thanks ! It should really improve the speed of some execution.

juergenpabel commented 3 years ago

All good thanks ! It should really improve the speed of some execution.

Yes, except for the first invocation. I am already thinking about implementing a cache-warmup of sort at startup, but haven't quite settled on a proper implementation path.

A concrete example: preloading of the vosk stt plugin only makes sense if the model is also loaded, thus requiring the proper parameters; this would probably make most sense in form of a python classmethod init() or alike - essentially a new API for dynamically loadable modules); however, different module types have no common base class (NeuronModule / SpeechRecognition / TTSModule / None for Trigger);

All in all it seems to me that a refactoring in the core should be done first:

Any thoughts?