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

Lazy import root systems, crystals and other combinatorics features to save on startup time. #15293

Open
nthiery opened this issue Oct 16, 2013 · 2 comments · May be fixed by #35520
Open

Lazy import root systems, crystals and other combinatorics features to save on startup time. #15293

nthiery opened this issue Oct 16, 2013 · 2 comments · May be fixed by #35520

Comments

@nthiery
Copy link
Contributor

nthiery commented Oct 16, 2013

The title says it all. See also #14652.

The attached patch is but a proof of concept to evaluate how much we
can hope to gain. And that's a rough 6% which is not so bad :-)

Without patch:

Total time (sum over exclusive time): 2871.367ms
Total time (sum over exclusive time): 2881.301ms
Total time (sum over exclusive time): 2876.823ms
Total time (sum over exclusive time): 2859.830ms
Total time (sum over exclusive time): 2865.218ms
Total time (sum over exclusive time): 2867.298ms
Total time (sum over exclusive time): 2864.525ms
Total time (sum over exclusive time): 2919.009ms

With patch:

Total time (sum over exclusive time): 2685.894ms
Total time (sum over exclusive time): 2691.945ms
Total time (sum over exclusive time): 2687.476ms
Total time (sum over exclusive time): 2681.121ms
Total time (sum over exclusive time): 2708.517ms
Total time (sum over exclusive time): 2688.468ms
Total time (sum over exclusive time): 2684.715ms

I haven't checked yet that all test pass properly, and this patch
probably conflicts with other patches. So no rush to get it in, the
goal is just to evaluate whether we can make up for the little startup
time loss with #10963.

A strange thing is that, from reading the startup report, most of the
progress comes from lazy importing the module
sage/combinat.abstract_tree which does not seem that particular.

The dependency upon #10963 is probably just syntactic; it might actually work without it.

Depends on #10963

CC: @sagetrac-sage-combinat @simon-king-jena @tscrim

Component: performance

Keywords: startup

Work Issues: Check that all test pass, check it does not conflict with other patches.

Author: Nicolas M. Thiéry

Issue created by migration from https://trac.sagemath.org/ticket/15293

@nthiery nthiery added this to the sage-6.1 milestone Oct 16, 2013
@nthiery
Copy link
Contributor Author

nthiery commented Oct 16, 2013

@fchapoton
Copy link
Contributor

Changed keywords from none to startup

@sagetrac-vbraun-spam sagetrac-vbraun-spam mannequin modified the milestones: sage-6.1, sage-6.2 Jan 30, 2014
@sagetrac-vbraun-spam sagetrac-vbraun-spam mannequin modified the milestones: sage-6.2, sage-6.3 May 6, 2014
@sagetrac-vbraun-spam sagetrac-vbraun-spam mannequin modified the milestones: sage-6.3, sage-6.4 Aug 10, 2014
@mkoeppe mkoeppe removed this from the sage-6.4 milestone Dec 29, 2022
mkoeppe pushed a commit to mkoeppe/sage that referenced this issue Apr 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants