blurb: don't overwrite output when not necessary#403
blurb: don't overwrite output when not necessary#403JulienPalard merged 2 commits intopython:masterfrom
Conversation
|
Poke @JulienPalard |
|
Example of speed up gained from the patch Without this patch, building the French translation after changing one With this patch, it only takes 12. |
|
How did you tested this? I'm testing it using: and I'm getting similar results with master vs your branch: vs |
|
(You can rebase on top of master if you want the CI to have a chance to pass) |
The speedup comes later. With this change, blurb won't update the file unnecessarily, which means Sphinx will then use a cache instead of parsing it again. |
|
Thanks @encukou, that's a perfect description of what I was trying to achieve :) |
462ef49 to
e085df3
Compare
|
Rebased but I need an approval for the workflow to run. (BTW, thanks for the CI fix) |
There was a problem hiding this comment.
The changes look good and I get the same output as with the current blurb.
This does load the entire NEWS file into memory twice, but I don't think that's an issue nowadays.
So why Also got this today while trying: |
No clue. Measuring performance is hard ... But as stated above, the goal is to speed up the sphinx build when blurb is called, not blurb itself.
I used this |
|
I'm testing with |
Oh you mean to avoid cache thinking the file has changed by not touching it? Got it now \o/ I had the feeling I was not understanding something ... I was not understanding something! Can you please just double check the exception on |
Me neither - but I can't reproduce the exception. This is weird. If you're afraid of breakage, we could push a beta version of blurb on pypi and check nothing breaks, no ? |
|
I'd look at it on my machine, as I can reproduce it, but not right now, I'm attending the trainers summit... I only have half my brain available :D Enough to run hyperfine, not enough to read the code :D |
|
OK my reproducer is: which make sense: if the file does not exists, you can't compare it. what about: new_contents = buff.getvalue()
try:
previous_contents = Path(output).read_text(encoding="UTF-8")
except (FileNotFoundError, UnicodeError):
previous_contents = None
if new_contents != previous_contents:
Path(output).write_text(new_contents, encoding="UTF-8")
else:
builtins.print(output, "is already up to date") |
|
Yup, that sounds better :) |
This speeds up doc generation. With this change, blurb won't update the NEWS file unnecessarily, which means Sphinx will then use a cache instead of parsing it again.
e085df3 to
cf225d6
Compare
|
Thanks a lot @dmerejkowsky! |
This speeds up the build of the Python documentation
quite a bit, since the NEWS file is huge and causes
sphinx to rebuild its doctree each time the doc
is built, regardless of the other changes