D6594: RFC dirstatemap

[Alphare (Raphaël Gomès)]

Alphare added a comment.
Alphare marked 4 inline comments as done.


  > Do you have some benchmark number compared to simpler (and dumb) approaches
  > such as caching PyList/Dict representation?
  
  I don't have any benchmarks as of yet, sorry.
  
  > I heard boxing a PyObject has measurable cost,
  
  Do you have a link to any benchmarks? I'd be interested.
  
  > so we might even want to keep the entire data backed by PyObjects 
  > depending on how frequently the data will be exposed to Python world.
  
  How would we decouple hg-core and hg-cpython then? If Python expects references to big objects (within `propertycache` for instance), we still need to exchange a fair bit of data from Rust if Rust does the heavy lifting.
  
  > (I don't have expertise to comment on the soundness of the proposed ref handling
  > business, and this patch is too big for me to review, sorry.)
  
  I understand. I am planning to split the change into smaller bits, but I was hoping to get some advice beforehand to reduce noise and history management work. You already have helped me a lot, so has @kevincox.

INLINE COMMENTS

> kevincox wrote in dirstate_map.rs:245
> This function seems quite odd. Why isn't dirs always set? It seems like you do some initialization then set it. However does this mean that this class has a different set of allowed methods before and after dirs is set?
> 
> If this is the case you should either A) Add assertions at the top of every method to ensure they are only called at allowed times or B) split out the initialization into a builder type.
> 
> Otherwise why can't `dirs` be set at initialization.

I agree that this pattern is quite unsatisfactory. The Python implementation uses a lot of lazy properties to omit some sub data structures until (or unless) they are really needed. This proved quite awkward to replicate, but there has to be a better solution to this problem. 
Your idea of using a builder pattern is tempting, but I don't know how it'll play out with Python, I will have to try.

REPOSITORY
  rHG Mercurial

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

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

To: Alphare, #hg-reviewers
Cc: yuja, durin42, kevincox, mjpieters, mercurial-devel