Optimize object filename from source #14200
Conversation
There was a missing parameter in one call. This was caused by b95e177
bruchar1
force-pushed
the
optimize-object-filename-from-source
branch
from
68349f1 to
a49b268
Compare
|
I don't understand the mypy failure here... |
It seems that EDIT: actually the stubs are right in |
mesonbuild/backend/backends.py
Outdated
| parts = Path(fname).parts | ||
| if mesonlib.is_windows(): | ||
| fname = fname.replace('\\', '/') | ||
| parts = fname.split('/') |
There was a problem hiding this comment.
This function is already cached by fname, so this change will only ever affect one call per filename that gets canonicalized. Is Path() actually showing up in profiling here?
There was a problem hiding this comment.
In its performance improvement tracking issue, @bonzini was mentioning speed up object_filename_from_source . The canonicalize_filename call was using a significant time in that function, and pathlib stuff was the culprit.
That being said, object_filename_from_source is not really a hotpath when generating my project. I could drop that commit if you think it doesn't worth it.
There was a problem hiding this comment.
I'm not saying that object_filename_from_source isn't slow! I'm just trying to understand whether caching it was enough to remove it as a hotpath. If it isn't a hotpath, then using Path().parts is a bit more convenient due to avoiding platform-specific if/else.
That discussion didn't even mention pathlib being the cause. I would personally have assumed that hashlib.sha1() is the cause, if I had to take a wild guess, though.
There was a problem hiding this comment.
When profiling my project, about half of the time in object_filename_from_source is spent in relpath, and maybe 20% in canonicalize_filename. In canonicalize_filename, half of the time is spent in Path.parts, and 20% in Path.__init__. I'm not sure I ever hit the sha1 call here.
Globally, I got 670 call to canonicalize_filename, and 27 calls to sha1, and time spent in sha1 is almost nothing.
| def _get_trials_from_pattern(cls, pattern: str, directory: str, libname: str) -> T.List[Path]: | ||
| f = Path(directory) / pattern.format(libname) | ||
| def _get_trials_from_pattern(cls, pattern: str, directory: str, libname: str) -> T.List[str]: | ||
| f = os.path.join(directory, pattern.format(libname)) |
There was a problem hiding this comment.
This change overall looks good. There was never any real reason to use pathlib here especially since we spent a lot of time transforming strings into Path objects just to reconvert them back into strings.
|
Note that #14109 added caching for canonicalize_filename, is it still a hot path? (The changes are all very nice anyway). |
Using str.split is faster than Path.parts
bruchar1
force-pushed
the
optimize-object-filename-from-source
branch
from
a49b268 to
2f8ea40
Compare
With this improvement, I observe a 10x speedup on the For instance, profiling my project, the cumulative time for
|
Inspired by #14103, I did some profiling on my project generation, and added some optimisations. The most significant is the _get_file_from_list() that cut generation time from 60 sec to 45 sec on my project.