This is an archive of the discontinued Mercurial Phabricator instance.

censor: use a reasonable amount of memory
ClosedPublic

Authored by valentin.gatienbaron on Sep 13 2018, 4:31 PM.

Details

Summary

Before this change, trying to censor some random revision uses an ever
increasing amount of memory (I stopped at 20GB, but it was by no means
finished), presumably because these contexts have a lot of
information that is kept alive.
After this change, the memory usage plateaus quickly.

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

This effectively changes things from a list to a generator. That means we can only iterate headctxs once.

We do only iterate headctxs once, so this is safe. But it isn't a good practice to leave a generator variable around in the local scope.

Perhaps we could rewrite this as a normal for loop that iteratively builds up heads using .append().

This effectively changes things from a list to a generator. That means we can only iterate headctxs once.
We do only iterate headctxs once, so this is safe. But it isn't a good practice to leave a generator variable around in the local scope.
Perhaps we could rewrite this as a normal for loop that iteratively builds up heads using .append().

I'm honestly fine with this change as-stated. Would you feel better if it was called headctxsgen instead?

(the for-loop-and-append is probably slightly slower, but I haven't benchmarked it)

This revision was automatically updated to reflect the committed changes.