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

DeferLoad class does not cache resolved classes as expected #3360

Closed
ff137 opened this issue Nov 28, 2024 · 0 comments · Fixed by #3361
Closed

DeferLoad class does not cache resolved classes as expected #3360

ff137 opened this issue Nov 28, 2024 · 0 comments · Fixed by #3361

Comments

@ff137
Copy link
Contributor

ff137 commented Nov 28, 2024

In testing out with some more logging added to the ClassLoader (#3332), I could see that /status checks would prompt the AskarStorage class to be reloaded each time. I noticed this comes from the DeferLoad class, so added logs as follows:

# Existing code, with extra debug log lines
class DeferLoad:
    """Helper to defer loading of a class definition."""

    def __init__(self, cls_path: str):
        """Initialize the `DeferLoad` instance with a qualified class path."""
        self._cls_path = cls_path
        self._inst = None

    def __call__(self, *args, **kwargs):
        """Magic method to call the `DeferLoad` as a function."""
        LOGGER.debug("Calling deferred class load for: %s", self._cls_path)
        return (self.resolved)(*args, **kwargs)

    @property
    def resolved(self):
        """Accessor for the resolved class instance."""
        LOGGER.debug("Resolving deferred class load for: %s", self._cls_path)
        if not self._inst:
            LOGGER.debug("Class not yet loaded, loading: %s", self._cls_path)
            self._inst = ClassLoader.load_class(self._cls_path)
            LOGGER.debug("Successfully loaded class: %s", self._cls_path)
        return self._inst

Results in logs as follows:

acapy_agent.admin.server DEBUG Incoming request: GET /status/live
acapy_agent.admin.server DEBUG Match info: <MatchInfo {}: <ResourceRoute [GET] <PlainResource  /status/live> -> <function liveliness_handler at 0x7f2b83f17a60>>
acapy_agent.admin.server DEBUG Body: None
acapy_agent.admin.server DEBUG Incoming request: GET /status/ready
acapy_agent.admin.server DEBUG Match info: <MatchInfo {}: <ResourceRoute [GET] <PlainResource  /status/ready> -> <function readiness_handler at 0x7f2b83f17ba0>>
acapy_agent.admin.server DEBUG Body: None
acapy_agent.utils.classloader DEBUG Calling deferred class load for: acapy_agent.storage.askar.AskarStorage
acapy_agent.utils.classloader DEBUG Resolving deferred class load for: acapy_agent.storage.askar.AskarStorage
acapy_agent.utils.classloader DEBUG Class not yet loaded, loading: acapy_agent.storage.askar.AskarStorage
acapy_agent.utils.classloader DEBUG Successfully loaded class: acapy_agent.storage.askar.AskarStorage
acapy_agent.utils.classloader DEBUG Calling deferred class load for: acapy_agent.storage.askar.AskarStorage
acapy_agent.utils.classloader DEBUG Resolving deferred class load for: acapy_agent.storage.askar.AskarStorage
acapy_agent.utils.classloader DEBUG Class not yet loaded, loading: acapy_agent.storage.askar.AskarStorage
acapy_agent.utils.classloader DEBUG Successfully loaded class: acapy_agent.storage.askar.AskarStorage

and repeats upon each status check.

This indicates that the self._inst = ClassLoader.load_class(self._cls_path) line does not persist the class for future re-use, and the load_class logic is executed each time.

This is presumably because a new instance of the DeferLoad class is created each time it is called. I can't be completely sure, but I guess it's that when a DeferLoad instance is called, a new one is being instantiated each time, and not being re-used. So the persistence to self._inst doesn't work as intended.

I found that adding class caching as follows is sufficient to solve this:

class DeferLoad:
    """Helper to defer loading of a class definition."""

    _class_cache = {}  # Shared cache for resolved classes

    def __init__(self, cls_path: str):
        """Initialize the `DeferLoad` instance with a qualified class path."""
        self._cls_path = cls_path

    def __call__(self, *args, **kwargs):
        """Magic method to call the `DeferLoad` as a function."""
        LOGGER.debug("Calling deferred class load for: %s", self._cls_path)
        return self.resolved(*args, **kwargs)

    @property
    def resolved(self):
        """Accessor for the resolved class instance."""
        LOGGER.debug("Resolving deferred class load for: %s", self._cls_path)
        if self._cls_path not in DeferLoad._class_cache:
            LOGGER.debug("Class not yet loaded, loading: %s", self._cls_path)
            DeferLoad._class_cache[self._cls_path] = ClassLoader.load_class(
                self._cls_path
            )
            LOGGER.debug("Successfully loaded class: %s", self._cls_path)
        return DeferLoad._class_cache[self._cls_path]

The above is applied in #3361 (without the debug logs)

ff137 added a commit to didx-xyz/acapy that referenced this issue Nov 28, 2024
ff137 added a commit to didx-xyz/acapy that referenced this issue Nov 28, 2024
@ff137 ff137 closed this as completed in 389e9d2 Dec 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant