Opened 4 years ago

Closed 4 years ago

Last modified 4 years ago

#31692 closed Cleanup/optimization (fixed)

compilemessages needlessly runs msgfmt on unchanged .po files

Reported by: Per Cederqvist Owned by: Claude Paroz
Component: Core (Management commands) Version: dev
Severity: Normal Keywords:
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

I have a project where running django-admin compilemessages takes 1.75 seconds. Running it again, when all the .mo files already exists and are up-to-date, also takes 1.75 seconds.

I propose that compilemessages.py is changed so that it only invokes msgfmt when it would do anything useful. This can be implemented by checking the mtime of the .po file and the corresponding .mo file. (If statting the .mo file fails, treat that as if the mtime was 0.) Only submit the command to the executor if the mtime of the .po file is greater than that of the .mo file. In effect: don't do anything if the .mo file is newer than the .po file.

There is one issue with this: the way the code currently uses the is_writable function. Since it modifies the mtime of the .mo file, you would have to perform the stat of the .mo file before you check if it is writable. (Or, you could just remove the is_writable function and its use. That feature is, in my opinion, of dubious value, and it doesn't appear to be documented.)

After I made the changes above, the runtime in the common case where nothing needs to be done was reduced from 1.75 seconds to 0.2 seconds.

(Unfortunately, I doubt that I will be able to get a Corporate Contributor License Agreement signed, so I can unfortunately not contribute my change.)

1.75 seconds may not be much, but when a CI system does it repeatedly, it adds up.

Change History (6)

comment:1 by Claude Paroz, 4 years ago

Triage Stage: UnreviewedAccepted

Proposal makes sense.

About the CLA, I don't remember one time where we refused a contribution because of that (should be time to drop it?).

comment:2 by Claude Paroz, 4 years ago

Owner: changed from nobody to Claude Paroz
Status: newassigned

comment:3 by Claude Paroz, 4 years ago

Has patch: set

comment:4 by Mariusz Felisiak <felisiak.mariusz@…>, 4 years ago

In ed0a040:

Refs #31692 -- Updated compilemessages and tests to use pathlib.

comment:5 by Mariusz Felisiak <felisiak.mariusz@…>, 4 years ago

Resolution: fixed
Status: assignedclosed

In e62d55a4:

Fixed #31692 -- Prevented unneeded .po file compilation.

Thanks Nick Pope and Simon Charette for the reviews.

comment:6 by GitHub <noreply@…>, 4 years ago

In 02ea98bc:

Refs #31692 -- Fixed compilemessages crash on Windows with Python < 3.8.

Regression in ed0a040773f5bad187170ab4e3b094fe3108d702.
See https://bugs.python.org/issue31961

Note: See TracTickets for help on using tickets.
Back to Top