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

start-up performance improvements #8221

Merged
merged 11 commits into from
May 17, 2024
Merged

start-up performance improvements #8221

merged 11 commits into from
May 17, 2024

Conversation

wardi
Copy link
Contributor

@wardi wardi commented May 8, 2024

Fixes #8219

Proposed fixes:

  • call plugins_update only when plugins changed (~25% reduction)
  • skip make_app for --help (~50% reduction)
  • update js translations only from ckan run and ckan translate js (~2% reduction)
  • use msgspec for faster yaml parsing

Remaining time spent:

flowchart TD
    MA["make_app (58%)"]
    LE["load_environment (42%)"]
    MFS["make_flask_stack (16%)"]
    UC["plugins_update (30%)"]
    Y["yaml parsing (13%)"]
    GV["get_validator (3%)"]
    MI["module imports (40%)"]
    MA --> LE --> UC --> Y
    MA --> MFS
    LE --> GV

Ideas:

  • lazy imports?
  • opt-out/in decorators for load_environment and make_flask_stack
  • cache get_validator
  • remove automatic build js translations

@smotornyuk
Copy link
Member

yaml parsing (13%)

With flask-session, CKAN now has implicit msgspec dependency. Even if we ignore all the cool possibilities of this library and focus only on really fast parsing, we can improve this number. Here are my local results for yaml and msgspec. For testing, I used config declarations from ckan/config/config_declaration.yaml.

In [10]: %timeit yaml.safe_load(open(filename, "rb").read())
154 ms ± 245 µs per loop (mean ± std. dev. of 7 runs, 10 loops each)

In [11]: %timeit msgspec.yaml.decode(open(filename, "rb").read())
10.4 ms ± 40.4 µs per loop (mean ± std. dev. of 7 runs, 100 loops each)

@wardi
Copy link
Contributor Author

wardi commented May 12, 2024

@smotornyuk great idea. Sounds like an easy fix to bring that 13% down to 1%.

@wardi
Copy link
Contributor Author

wardi commented May 12, 2024

@amercader I didn't see an easy way to add decorators with click for load_environment and make_flask that are backwards compatible, so I'm leaving it for now. This PR is ready for review.

xychart-beta horizontal
    title "CLI performance improvements"
    x-axis ["ckan user list", "NEW ckan user list", "ckan --help", "NEW ckan --help"]
    y-axis "Speed (invocations/s)" 0 --> 0.8
    bar [.313, .376, .317, .649]

@wardi
Copy link
Contributor Author

wardi commented May 12, 2024

failure looks unrelated, passes locally.

@wardi
Copy link
Contributor Author

wardi commented May 14, 2024

@amercader my preference with make_app and load_environment is to export them in the plugins toolkit and switch all the cli tools to call one of them only when necessary.

That would allow quick handling for catching errors in parameters etc. and involve no hacky special cases for --help or ckan generate config.

External extensions with cli interfaces can check for the existence of the make_app or load_environment and if they aren't there assume make_app has already been called to be compatible with old and new ckans.

WDYT?

changes/8219.feature Outdated Show resolved Hide resolved
Co-authored-by: Adrià Mercader <amercadero@gmail.com>
requirements.txt Outdated
@@ -1,5 +1,5 @@
#
# This file is autogenerated by pip-compile with Python 3.9
# This file is autogenerated by pip-compile with Python 3.10
Copy link
Member

Choose a reason for hiding this comment

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

It is a pain but we need to generate the requirements.txt file in a 3.9 env otherwise we might lose requirements needed there. Or run pip-compile -P msgspec requirements.in to just touch that package

@amercader amercader merged commit d77ceb7 into master May 17, 2024
6 of 8 checks passed
@amercader amercader deleted the 8219-start-up-performance branch May 17, 2024 11:16
@amercader amercader added this to the CKAN 2.11 milestone May 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

performance improvements: start-up time
3 participants