Optimize construction of Path from other Paths by just returning the same object?

[anntzer]

BPO	39783
Nosy	@brettcannon, @rhettinger, @pitrou, @remilapeyre, @barneygale
PRs	bpo-39783: Optimize contruction of Path and PurePath objects #20288

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = None
closed_at = None
created_at = <Date 2020-02-28.11:36:37.520>
labels = ['library', '3.10']
title = 'Optimize construction of Path from other Paths by just returning the same object?'
updated_at = <Date 2020-06-21.09:44:05.958>
user = 'https://github.com/anntzer'

bugs.python.org fields:

activity = <Date 2020-06-21.09:44:05.958>
actor = 'Antony.Lee'
assignee = 'none'
closed = False
closed_date = None
closer = None
components = ['Library (Lib)']
creation = <Date 2020-02-28.11:36:37.520>
creator = 'Antony.Lee'
dependencies = []
files = []
hgrepos = []
issue_num = 39783
keywords = ['patch']
message_count = 6.0
messages = ['362874', '362884', '362886', '362887', '362919', '362940']
nosy_count = 5.0
nosy_names = ['brett.cannon', 'rhettinger', 'pitrou', 'remi.lapeyre', 'barneygale']
pr_nums = ['20288']
priority = 'normal'
resolution = None
stage = 'patch review'
status = 'open'
superseder = None
type = None
url = 'https://bugs.python.org/issue39783'
versions = ['Python 3.10']

[anntzer]

Many functions which take a path-like object typically also accept strings (sorry, no hard numbers here). This means that if the function plans to call Path methods on the object, it needs to first call Path() on the arguments to convert them, well, to Paths. This adds an unnecessary cost in the case where the argument is *already* a Path object (which should become more and more common as the use of pathlib spreads), as Path instantiation is not exactly cheap (it's on the order of microseconds).

Instead, given that Paths are immutable, Path(path) could just return the exact same path instance, completely bypassing instance creation (after checking that the argument's type exactly matches whatever we need and is not, say, PureWindowsPath when we want to instantiate a PosixPath, etc.). Note that there is prior art for doing so in CPython: creating a frozenset from another frozenset just returns the same instance:

In [1]: s = frozenset({1}); id(s) == id(frozenset(s)) == id(s.copy())
Out[1]: True

[rhettinger]

Possibly this could be done just by adding @lru_cache to the __new__() method.

[remilapeyre]

There is the _closed attribute thought that is mutated by some methods.

[anntzer]

Decorating __new__ with lru_cache would likely run into memory leakage problems? (one would need a "weak lru_cache", I guess).

I didn't know about the _closed attribute. From a quick look it appears to only be settable by using the path (not the actual file) as a context manager, and only serves to block further filesystem methods, but I'm not even sure why? (for example, it doesn't block fspath, so it won't prevent operations via os.path functions anyways...)

[remilapeyre]

Decorating __new__ with lru_cache would likely run into memory leakage problems?

I think the LRU cache would be for returning the same instance when called with the same string. I don't think it would be needed to return the same instance when called with a Path instance.

From a quick look it appears to only be settable by using the path (not the actual file) as a context manager, and only serves to block further filesystem methods, but I'm not even sure why?

This came up in bpo-39682 recently, I'm not sure what this flag is supposed to mean and I could not find it in the documentation.

Since it's uncorrelated to the file object and the physical file, I think it may actually be dangerous to rely on this flag. I don't know why is the purpose of using Path as a context manager.

[barneygale]

Attempted fix shown here: barneygale@784630e

Note that I've already removed context manager support (and hence _closed) in this branch.