Skip to content
Snippets Groups Projects

Draft: obsolete: make content-divergence() be more selective in the parent set

Merge request reports

Loading
Loading

Activity

Filter activity
  • Approvals
  • Assignees & reviewers
  • Comments (from bots)
  • Comments (from users)
  • Commits & branches
  • Edits
  • Labels
  • Lock status
  • Mentions
  • Merge request status
  • Tracking
  • requested review from @mercurial.review

  • Raphaël Gomès mentioned in merge request !863 (merged)

    mentioned in merge request !863 (merged)

  • added 1 commit

    • 8269743a99e9 - obsolete: make content-divergence() be more selective in the parent set

    Compare with previous version

  • added 1 commit

    • 7b829e1ccb12 - obsolete: make content-divergence() be more selective in the parent set

    Compare with previous version

  • added 1 commit

    • 7b829e1ccb12 - obsolete: make content-divergence() be more selective in the parent set

    Compare with previous version

  • Author Mercurial Publisher

    For hg log -r 'contentdivergent()' on NetBSD src, I get:

    Before:

    real    0m0.768s
    user    0m0.684s
    sys     0m0.084s
    
    real    0m0.750s
    user    0m0.673s
    sys     0m0.077s
    
    real    0m0.773s
    user    0m0.679s
    sys     0m0.093s

    After:

    real    0m0.382s
    user    0m0.339s
    sys     0m0.043s
    
    real    0m0.395s
    user    0m0.325s
    sys     0m0.070s
    
    real    0m0.400s
    user    0m0.356s
    sys     0m0.043s
    Edited by Jörg Sonnenberger
  • added 1 commit

    • 7b829e1ccb12 - obsolete: make content-divergence() be more selective in the parent set

    Compare with previous version

  • Author Mercurial Publisher

    The failing test case seems to be a newly exposed real bug in the way strip and obsolescence interact. When stripping non-tip, strip operates in two separate transactions. The first transaction bundles up changesets to be preserved and truncates the files affected. The second transaction will (re)apply the changesets preserved in the first step. Now in the problematic test case, . is stripped indirectly and reapplied in the second transaction. But it means that the dirstate is invalid after the first transaction. With the changed computation for the content-divergence, it doesn't depend solely on pre-cached data, but actually hits localrepository._dirstatevalidate in mercurial/localrepo.py:1853. One possible workaround (which seems sensible in any case) would be suppress reporting divergence at the end of the first transaction.

  • Split does not create divergence, so they could be ignored in you filtering.

    However I am not convinced by the tradeoff here. In general, the amount of obsmarker will grow unbounded, while the number of visible draft stay more or less under control. So changing something that is O(|draft|) into something O(|obsmarker|) does not seems like a good idea. I believe the current computation could be greatly improved, and I suspect using your prefiltering could be used with it.

    Suppressing divergence at the first of the first transaction seems fine. (but having a join report at the end of the second transaction seems important then ?)

  • Author Mercurial Publisher

    The current implementation scales in both dimensions as we always pay the time to parse the obsmarkers already. Especially for repositories with many draft changes compared to the number of obsmarkers, the current implementation is expensive. That said, the real fix is likely to fix the performance of the successorset computation itself.

  • we always pay the time to parse the obsmarkers already.

    The current format is obviously a problem in that regard that will get fixed sooner or later.

    Please strop using it as a excuse to inject more O(|obsmarkers|) complexity in that thing :-)

    Can you send a version of this series that re-use the idea that speed up the processing, without diverging the current O(|draft|) approach we currently have ? (or use another one, but not O(|obsmarkers|)

  • Pierre-Yves David marked this merge request as draft

    marked this merge request as draft

  • Moving back to draft until we have a new version or a voice discussion about this.

  • requested review from @mercurial.review

Please register or sign in to reply
Loading