[Python-Dev] [Python-checkins] cpython: Issue #18810: Be optimistic with stat calls when seeing if a directory

[Nick Coghlan]

On 19 Oct 2013 03:24, "brett.cannon" <python-checkins at python.org> wrote:
>
> http://hg.python.org/cpython/rev/11f2f4af1979
> changeset:   86444:11f2f4af1979
> user:        Brett Cannon <brett at python.org>
> date:        Fri Oct 18 13:24:13 2013 -0400
> summary:
>   Issue #18810: Be optimistic with stat calls when seeing if a directory
> exists when checking for a package.
>
> Before there was an isdir check and then various isfile checks for
> possible __init__ files when looking for a package.
> This change drops the isdir check by leaning
> on the assumption that a directory will not contain something named
> after the module being imported which is not a directory. If the module
> is a package then it saves a stat call. If there is nothing in the
> directory with the potential package name it also saves a stat call.
> Only if there is something in the directory named the same thing as
> the potential package will the number of stat calls increase
> (due to more wasteful __init__ checks).

I don't follow this logic. There's now not even an existence check for the
base name, so it reads to me like we will look for all the possible init
file extensions even if there's *no* directory with an appropriate name.

What am I missing?

Cheers,
Nick.

>
> Semantically there is no change as the isdir check moved
> down so that namespace packages continue to have no chance of
> accidentally collecting non-existent directories.
>
> files:
>   Lib/importlib/_bootstrap.py |    19 +-
>   Python/importlib.h          |  1537 +++++++++++-----------
>   2 files changed, 777 insertions(+), 779 deletions(-)
>
>
> diff --git a/Lib/importlib/_bootstrap.py b/Lib/importlib/_bootstrap.py
> --- a/Lib/importlib/_bootstrap.py
> +++ b/Lib/importlib/_bootstrap.py
> @@ -1406,16 +1406,15 @@
>          # Check if the module is the name of a directory (and thus a
package).
>          if cache_module in cache:
>              base_path = _path_join(self.path, tail_module)
> -            if _path_isdir(base_path):
> -                for suffix, loader in self._loaders:
> -                    init_filename = '__init__' + suffix
> -                    full_path = _path_join(base_path, init_filename)
> -                    if _path_isfile(full_path):
> -                        return (loader(fullname, full_path), [base_path])
> -                else:
> -                    # A namespace package, return the path if we don't
also
> -                    #  find a module in the next section.
> -                    is_namespace = True
> +            for suffix, loader in self._loaders:
> +                init_filename = '__init__' + suffix
> +                full_path = _path_join(base_path, init_filename)
> +                if _path_isfile(full_path):
> +                    return (loader(fullname, full_path), [base_path])
> +            else:
> +                # If a namespace package, return the path if we don't
> +                #  find a module in the next section.
> +                is_namespace = _path_isdir(base_path)
>          # Check for a file w/ a proper suffix exists.
>          for suffix, loader in self._loaders:
>              full_path = _path_join(self.path, tail_module + suffix)
> diff --git a/Python/importlib.h b/Python/importlib.h
> --- a/Python/importlib.h
> +++ b/Python/importlib.h
> [stripped]
>
> --
> Repository URL: http://hg.python.org/cpython
>
> _______________________________________________
> Python-checkins mailing list
> Python-checkins at python.org
> https://mail.python.org/mailman/listinfo/python-checkins
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.python.org/pipermail/python-dev/attachments/20131019/a1e92507/attachment.html>