Skip to content

perf: Refine the Model Manager code #3098

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

Merged
merged 1 commit into from
May 16, 2025
Merged

Conversation

shaohuzhang1
Copy link
Contributor

perf: Refine the Model Manager code

@shaohuzhang1 shaohuzhang1 merged commit b0630b3 into main May 16, 2025
4 checks passed
@shaohuzhang1 shaohuzhang1 deleted the pr@main@perf_model_manage branch May 16, 2025 04:09
rsa_value = get_key_pair_by_sql()
rsa_cache.set(cache_key, rsa_value)
finally:
lock.release()
return rsa_value


Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is no error in this code, but there is an inefficiency in obtaining keys. If a key pair already exists in the cache, it does not need to fetch one from the database each time because rsa_cache.set(cache_key, rsa_value) will always overwrite the existing value in the cache even if the key pair hasn't changed.

To optimize this function, you can modify it so that only calls to get_key_pair_by_sql() actually fetch data from the database:

def generate():
    def get_key_pair():
        rsa_value = rsa_cache.get(cache_key)
        if rsa_value is None:
            with lock:
                rsa_value = rsa_cache.get(cache_key)
                if rsa_value is not None:
                    return rsa_value
                # Fetch keys from database only if they're missing.
                rsa_value = get_key_pair_by_sql() if not has_new_keys else None
                if rsa_value is not None:
                    rsa_cache.set(cache_key, rsa_value)
                    has_new_keys = True  # Reset flag after adding new values.
        return rsa_value

    has_new_keys = False
    generate()

If we know in advance when a full update is likely (for instance, during system startup), has_new_keys could be set once at the start of application initialization phase instead of checking every access to generate. This avoids unnecessary queries while still providing quick response on demand unless a full refresh might be needed sooner due to some event.

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 this pull request may close these issues.

1 participant