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 _______________________________________________ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
