Page MenuHomePhabricator

engine: prevent a function call for each store file
ClosedPublic

Authored by pulkit on Thu, Dec 31, 11:42 AM.

Details

Summary

Python function calls are not cheap. Instead of calling the function once for
each file in store, we use a dedicated function which filters and yields the
required files.

Diff Detail

Repository
rHG Mercurial
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

pulkit created this revision.Thu, Dec 31, 11:42 AM
marmoute accepted this revision.Thu, Jan 7, 9:39 PM

on second though, I am not sure what's the motivation is here. How long does filtering all entry actually takes ?

on second though, I am not sure what's the motivation is here. How long does filtering all entry actually takes ?

I didn't perf-measure it. Earlier it used to do a function call for each file inside .hg/store and now it's just one function call.

yeah, but that function itself is doing 1 call per file, so I doubt we will be a significant performance win.

mharbison72 requested changes to this revision.Tue, Jan 12, 6:39 PM
mharbison72 added a subscriber: mharbison72.
mharbison72 added inline comments.
mercurial/upgrade_utils/engine.py
378–379

Accidental change?

This revision now requires changes to proceed.Tue, Jan 12, 6:39 PM
mharbison72 accepted this revision.Tue, Jan 12, 6:45 PM
This revision is now accepted and ready to land.Tue, Jan 12, 6:45 PM
mharbison72 requested changes to this revision.Tue, Jan 12, 6:47 PM

Oops, typo on my end with phabupdate. I still wanted to flag the new return statement.

This revision now requires changes to proceed.Tue, Jan 12, 6:47 PM

yeah, but that function itself is doing 1 call per file, so I doubt we will be a significant performance win.

Hm, no? Now we have one function which yields results. Does yielding call function multiple times?

mercurial/upgrade_utils/engine.py
378–379

Ha yep!

mharbison72 accepted this revision.Wed, Jan 13, 12:11 PM
This revision is now accepted and ready to land.Wed, Jan 13, 12:11 PM
This revision was automatically updated to reflect the committed changes.