Skip to content

Cache package names in create_source_list #4848

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 3 commits into from
Apr 4, 2018
Merged

Conversation

msullivan
Copy link
Collaborator

On my testing MBP, this shaves about 200ms off of every fine-grained check of S.

@msullivan msullivan requested review from JukkaL and gvanrossum April 3, 2018 20:57
Copy link
Member

@gvanrossum gvanrossum left a comment

Choose a reason for hiding this comment

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

Let's refactor this so that crawl_up() etc. are methods on a new class that owns the package cache and wraps the fscache.


parent_dir, base = os.path.split(dir)
if not dir or not get_init_file(fscache, dir) or not base:
return ''
Copy link
Member

Choose a reason for hiding this comment

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

So this result should not be cached?

Copy link
Member

@gvanrossum gvanrossum left a comment

Choose a reason for hiding this comment

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

Just two nits.

mod = base
else:
mod = base + '.' + mod
Use package_cache to cache results."""
Copy link
Member

Choose a reason for hiding this comment

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

Move the """ to the next line (by itself). There's no need for a subsequent blank line then.


def expand_dir(self, arg: str, mod_prefix: str = '') -> List[BuildSource]:
"""Convert a directory name to a list of sources to build."""
f = get_init_file(self.fscache, arg)
Copy link
Member

Choose a reason for hiding this comment

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

Perhaps make get_init_file() also a method here? (Since it uses fscache.)

@msullivan msullivan merged commit 6b05cb7 into master Apr 4, 2018
@msullivan msullivan deleted the find_sources_cache branch April 4, 2018 18:16
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.

2 participants