D8110: rust-dirstatemap: cache non normal and other parent set

2020-02-14 Thread Raphaël Gomès
Alphare added inline comments. INLINE COMMENTS > kevincox wrote in dirstate_map.rs:251 > It works fine for me: > https://play.rust-lang.org/?version=stable=debug=2018=f74f3771641d1e29bdd4de1444c43324 Aaah I still had ``, and the auto-deref rules didn't work there. I see that this is much

D8110: rust-dirstatemap: cache non normal and other parent set

2020-02-14 Thread kevincox (Kevin Cox)
kevincox added inline comments. INLINE COMMENTS > Alphare wrote in dirstate_map.rs:251 > `as_mut()` only does ` Option -> Option< T>`, which isn't always > what we want (however you can argue that it would save a few keystrokes), but > calling `unwrap()` circles back to the same `error[E0515]:

D8110: rust-dirstatemap: cache non normal and other parent set

2020-02-14 Thread Raphaël Gomès
Alphare added inline comments. INLINE COMMENTS > kevincox wrote in dirstate_map.rs:251 > You should be able to use `.as_mut()` just like you are currently doing in > the caller. `as_mut()` only does ` Option -> Option< T>`, which isn't always what we want (however you can argue that it would

D8110: rust-dirstatemap: cache non normal and other parent set

2020-02-14 Thread kevincox (Kevin Cox)
kevincox added inline comments. INLINE COMMENTS > Alphare wrote in dirstate_map.rs:251 > Unwrapping creates a temporary value that is dropped instantly, so unless I'm > not seeing something, I don't think we can. You should be able to use `.as_mut()` just like you are currently doing in the

D8110: rust-dirstatemap: cache non normal and other parent set

2020-02-13 Thread Raphaël Gomès
Closed by commit rHG58c74a517a00: rust-dirstatemap: cache non normal and other parent set (authored by Alphare). This revision was automatically updated to reflect the committed changes. This revision was not accepted when it landed; it landed in state "Needs Review". REPOSITORY rHG Mercurial

D8110: rust-dirstatemap: cache non normal and other parent set

2020-02-13 Thread Raphaël Gomès
Alphare added inline comments. INLINE COMMENTS > kevincox wrote in dirstate_map.rs:38 > I don't understand why an `Option` is faster than a `HashSet`. Could > you add some explanation to the commit message? Is this to avoid attempting > to initialize the entry multiple times? Using `Option`

D8110: rust-dirstatemap: cache non normal and other parent set

2020-02-13 Thread Raphaël Gomès
Alphare added a comment. In D8110#120671 , @marmoute wrote: > This is supposed to be a graft of something already accepted on default. So unless I did a mistake on the graft (cc @Alphare for checking) any feedback on this also apply on the

D8110: rust-dirstatemap: cache non normal and other parent set

2020-02-13 Thread marmoute (Pierre-Yves David)
marmoute added a comment. marmoute added a subscriber: Alphare. This is supposed to be a graft of something already accepted on default. So unless I did a mistake on the graft (cc @Alphare for checking) any feedback on this also apply on the default one. REPOSITORY rHG Mercurial CHANGES

D8110: rust-dirstatemap: cache non normal and other parent set

2020-02-13 Thread kevincox (Kevin Cox)
kevincox added inline comments. INLINE COMMENTS > dirstate_map.rs:38 > +non_normal_set: Option>, > +other_parent_set: Option>, > parents: Option, I don't understand why an `Option` is faster than a `HashSet`. Could you add some explanation to the commit message? Is this to avoid

D8110: rust-dirstatemap: cache non normal and other parent set

2020-02-12 Thread marmoute (Pierre-Yves David)
marmoute created this revision. marmoute added a comment. Herald added subscribers: mercurial-devel, kevincox. Herald added a reviewer: hg-reviewers. INTENDED FOR STABLE REVISION SUMMARY Performance of `hg update` was significantly worse since the introduction of the Rust `dirstatemap`.