Stream: t-compiler/wg-incr-comp

Topic: Moving the `odht` crate to rust-lang?


view this post on Zulip mw (Mar 19 2021 at 10:57):

Dear @wg-incr-comp,

As some of you might have seen I've been working on simplifying and optimizing how incr. comp. handles DefPathHash to DefId mapping in https://github.com/rust-lang/rust/pull/82183. Both the reduction of complexity and the performance improvements look very promising -- so I'd like to move forward with getting the PR actually merged.

The main reason why the PR can simplify the implementation while at the same time improve performance is that it relies on a hash table implementation that does not need any upfront decoding. This is a very useful thing to have. With memory mapped files the compiler will even only load those parts of the hash table from the disk that are actually accessed. I imagine that we can find quite a few more use cases for a data structure like this.

The on-disk hash table is implemented in https://github.com/michaelwoerister/odht and, in accordance to https://rust-lang.github.io/compiler-team/procedures/crates/, I would like to propose moving this crate into the rust-lang org. I think the crate can be generally useful throughout the compiler and won't need a lot of maintenance once everything is set up.

What do you think?

view this post on Zulip cjgillot (May 09 2021 at 12:17):

I think #82183 is a great move forward, and that odht may eventually help with the hygiene interning table also. What is the required review process to move odht into rust-lang? I have been looking into the implementation, but I do not have a comprehensive understanding yet.

view this post on Zulip cjgillot (Jul 05 2021 at 19:52):

@mw What do you need to move this forward? Is a formal review process necessary?

view this post on Zulip mw (Jul 06 2021 at 09:22):

The MCP has been accepted so we just need to transfer the crate to the rust-lang org and do a crates.io release. It would indeed be helpful if someone could do a review of the crate. The fact that it had another pair of eyes on it could be documented in one of the first PRs that makes use of it, I'd say. Or even better, we add at least one co-owner to the Cargo.toml after the transfer.

@cjgillot, if you want to take a look at the code that would be very helpful indeed. It tried to add tests and documentation but it is certainly a good idea to read up on how SwissTable works (e.g. here: https://gankra.github.io/blah/hashbrown-tldr/). That will make it a lot easier to follow the code, I'm sure.

@Wesley Wiser, do you agree that that would be sufficient in terms of giving the crate an initial review?

view this post on Zulip Wesley Wiser (Jul 06 2021 at 13:46):

I think that would be fine. I can also take a look if you don't have time @cjgillot


Last updated: Oct 21 2021 at 22:01 UTC