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

Speed up dependency solving by using pool ids #1081

Merged
merged 5 commits into from Feb 24, 2020

Conversation

mlschroe
Copy link
Contributor

This gets rid of a couple of id->str->id roundtrips and also makes the dependency hashes in rpmtsCheck() use pool ids instead of strings.

This will be used to get rid of unneeded id->str->id roundtrips.
We already know the ids, so just use them for fingerprinting.
If the directories match the fingerprint is not used.
This needs less memory and it should also be faster as the
dependencies are already available in id form.
@mlschroe mlschroe added DONT DO NOT merge, for whatever reason and removed DONT DO NOT merge, for whatever reason labels Feb 21, 2020
@lgtm-com
Copy link

lgtm-com bot commented Feb 21, 2020

This pull request introduces 1 alert when merging d9ffe24 into 153c5c2 - view on LGTM.com

new alerts:

  • 1 for Implicit function declaration

The rpmalCreate() function is called in two places, in both of
them the arguments are all read from the transaction set.

So simplify the code by passing the ts to rpmalCreate() and
getting the values in the constructor.
Copy link
Member

@pmatilai pmatilai left a comment

Choose a reason for hiding this comment

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

I thought I actually removed the ts as an argument to rpmalCreate() back somewhen as part of overall efforts to minimize what has access to the full transaction set, but seems that's not the case. Anyway, I can live with that, it does of course simplify the argument list.

Nice optimization + cleanup, thanks.

@pmatilai pmatilai merged commit fafdc80 into rpm-software-management:master Feb 24, 2020
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.

None yet

2 participants