CHANGE
commit7b485eaae147fa3f26af06c0ba43cad6f46a5ef3
authorLucian Wischik <ljw@fb.com>
Sat, 10 Sep 2022 14:09:28 +0000 (10 07:09 -0700)
committerFacebook GitHub Bot <facebook-github-bot@users.noreply.github.com>
Sat, 10 Sep 2022 14:09:28 +0000 (10 07:09 -0700)
treea9f59dac47fadb149c0fac5c0485565d5e9c021e
parent172383325ab90c3a12f41e60f3699307616d1606
CHANGE

Summary:
This is the CHANGE action for inproc hh_decl_shmem.

There already exists an implementation for CHANGE in the outproc "decl-service" process. It uses a bunch of APIs from naming_sqlite.

I chose not to follow its algorithms nor many of its naming_sqlite::* helper algorithms. It started with me trying to reason about the code and not being able to prove its correctness. My failure to prove correctness led me directly to some examples which should have worked but instead crashed D38883904. I came to believe that the structure of the algorithm+helpers wasn't right...

The previous algorithm iterated over files and uses a method `naming_sqlite::insert_file_summary` which updates the forward naming table and then re-shuffles the reverse naming table+overflow as appropriate. What I've done instead is break it down into separate steps:
1. Iterate over files. Use this to update the forward naming table, but merely gather all symbol-changes and group them by symbol.
2. Iterate over symbols. This is an opportunity to calculate winners in an algorithm whose correctness is easier to see. Then we can write them into the reverse naming table.

I wrote lots of comments about invariants! And a few diffs up the stack, I wrote an exhaustive test of every possible non-overflow case.

I deliberately didn't yet do concurrency-control for the CHANGE action. That will be left for a future diff. I wanted to work solely on correctness for now.

Reviewed By: bobrenjc93

Differential Revision: D37417027

fbshipit-source-id: f2c44d2bb17236e463fdff990b867bd7debe39d5
hphp/hack/src/naming/names_rust/naming_sqlite.rs
hphp/hack/src/naming/names_rust/naming_table.rs