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

significantly speed up robot_find_subtoolchain_for_dep by specifying whether dep is resolved via an external module #2697

Merged
merged 1 commit into from Dec 17, 2018

Conversation

boegel
Copy link
Member

@boegel boegel commented Dec 15, 2018

I noticed that the easyconfigs test suite became significantly slower after #2690 got merged, so I did a profile run to figure out where most time was being spent...

It turns out that a whopping 75% of time was spent in the mod_exist_via_show inner function of ModulesTool.exist, which is only used as a fallback check in case a (visible) module is not found via the list produced by module avail.
This is required because a module name can be partial, in the case of external modules (cfr. https://easybuild.readthedocs.io/en/latest/Using_external_modules.html).

mod_exist_via_show is expensive since it involves actually calling out to the modules tool (e.g. Lmod) every time. We already have a "show cache", which helps quite a bit (it cuts down actual module show calls to with about 75% in the case of the easyconfigs test suite), but we still call module show once for every module name that is not found in the output of module avail.

Since robot_find_subtoolchain_for_dep knows when a dependency is resolved via an external module or not, it can pass down this information to ModulesTool.exist to avoid all these needless module show calls when we are certain that the module name is not partial (since that can only occur with dependencies marked as external modules).

It's worth mentioning that ModulesTool.exist already handles hidden module names separately, and so a fallback to show is already in place for module names for which the actual module file starts with a ..
For modules that are hidden via Lmod's feature to hide modules via .modulerc were also save, since we run module --show-hidden avail with Lmod, and so hidden modules will be listed.

This change has a dramatic effect on the time needed for easyconfig test suite: it was cut down from ~2550s to ~550s (on my laptop).

…whether dep is resolved via an external module
@boegel boegel added this to the 3.8.0 milestone Dec 15, 2018
@boegel boegel requested a review from ocaisa December 15, 2018 11:17
Copy link
Member

@ocaisa ocaisa left a comment

Choose a reason for hiding this comment

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

LGTM

@ocaisa
Copy link
Member

ocaisa commented Dec 17, 2018

Code looks fine, bit surprised that this didn't impact the times of the tests on Travis (they still take 10-14 mins).

@ocaisa
Copy link
Member

ocaisa commented Dec 17, 2018

@ocaisa
Copy link
Member

ocaisa commented Dec 17, 2018

Going in, thanks for reducing our while-that's-chugging-away-I'll-just-go-have-a-coffee window @boegel !

@ocaisa ocaisa merged commit bb210d1 into easybuilders:3.8.x Dec 17, 2018
@boegel boegel deleted the mod_exist_maybe_partial branch December 17, 2018 09:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants