D7144: status: use unfiltered repo if we're getting status of working copy

marmoute (Pierre-Yves David) phabricator at mercurial-scm.org
Thu Nov 14 10:13:06 EST 2019


marmoute added a comment.


  In D7144#108574 <https://phab.mercurial-scm.org/D7144#108574>, @martinvonz wrote:
  
  > In D7144#108444 <https://phab.mercurial-scm.org/D7144#108444>, @marmoute wrote:
  >
  >> I took more time to look at your series and at the bigger picture. My general position is still that passing the wrong filter level to high level function is hacking and will be a significant source of bugs.
  >
  > I agree that it's a hack.
  >
  >> For example, `hg status` taks a "PATTERN" argument, that pattern can be fileset, that fileset can contains revset. PAssing the wrong filtering will result in wrongdoing of that revset.
  >> However it might be a good idea to do this hack if:
  >>
  >> - the performance gain is significant,
  >> - the hack is temporary,
  >> - there is no reasonable alternative available short terms.
  >>
  >> I took some time to poke at the issues, and I managed to get all 4 commands listed in this series with a first throw of abotu 12 changesets. The idea is to recognise pattern that will not be filtered and to have lower level logic skip around the filtering in this case. So the higher level logic is still filtered with the right semantic. One can have a look at the very first throw here:  https://dev.heptapod.net/octobus/mercurial-devel/commits/topic/default/filter-trigger
  >
  > I did something similar (but much smaller in scope) a long time ago: https://www.mercurial-scm.org/repo/hg/rev/a9b61dbdb827. That hack has since become ineffective, and I think some of your commits aim at restoring its effect. I felt that that approach was also fragile since quite subtle code changes can make it stop working (as has already happened with my original hack). So I'm not convinced that is the right approach either, but I'm fine with taking that series instead if you clean it up (I do see the risk of bugs with my series).
  
  I think a consolidated approach will work reasonably well in most simpe case. However, yes, the approach is "fragile" since any other code directly touching `repo.changelog` would trigger the filtering. (We could probably improve that too).
  
  Since I am about to add a filtered repository to the benchmarks, we should be able to detect regression here. That's not ideal… but eventually we will also speed up the filtering itself.
  
  > Relatedly, I would really like to have the filtering applied only when walking towards children in the changelog (this is something that Durham has mentioned before),
  
  I think a generalisation of my current approach is close to that. If you walk parent from a revision that you know to exists, you don't need to check filtering.
  
  > so no filtering happens even if you do `hg status -r <some hidden commit>` or similar. But that's a much larger change, of course.
  >
  >> It is getting late so I will stop here for tonight. However, the prototype looks promissing enough. I can probably have a clean/consolidate version by the end of the week.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST ACTION
  https://phab.mercurial-scm.org/D7144/new/

REVISION DETAIL
  https://phab.mercurial-scm.org/D7144

To: martinvonz, #hg-reviewers, durin42
Cc: mharbison72, pulkit, marmoute, mercurial-devel


More information about the Mercurial-devel mailing list