Limit naming-table tendrils 1/2
commita986bec2760cb2f3a15b0e481cf5c04f387c6a1c
authorLucian Wischik <ljw@meta.com>
Tue, 6 Dec 2022 05:47:45 +0000 (5 21:47 -0800)
committerFacebook GitHub Bot <facebook-github-bot@users.noreply.github.com>
Tue, 6 Dec 2022 05:47:45 +0000 (5 21:47 -0800)
tree79d6e3a4de19967d32878476aae26bb1c370d5d5
parent18aad4725db99531a56e55f061a846257363928d
Limit naming-table tendrils 1/2

Summary:
Currently hh_server has tendrils into naming-table and decl-provider. I am trying to separate them, to make a clean abstraction called "hh_decl", which encapsulates the naming table as an internal detail hidden from callers, and whose chief purpose is blazing fast delivery of decls.

What do the naming-table tendrils look like? They're places where the code directly queries the naming-table for information, rather than getting everything it needs solely from decls. And then to support this, hh_server needs to build a naming-table cache, in addition to its decl cache. Except through misdesign it uses its shmem naming-table cache both for correctness (e.g. when naming table is updated) and for caching. That's part of why I want to get rid of the naming-table tendrils.

What would success look like?
1. Not needing the naming-table cache in the re-architecture. It's some code ugliness I'd like to avoid.
2. In the hot path (of typechecking code that has no typecheck errors) we should never depend upon that naming-table cache for speed. We should solely depend upon the existing decl shmem cache for speed. (It's fine to do slower naming-table reads in order to know how to render error messages).
3. Whatever naming-table APIs we use, must be exposed by hh_decl, since it owns the naming-table and tries to encapsulate it. I want to be sure I'm exposing only the bare minimum needed.

I approached this by writing a dummy diff D41254732 which raises an exception if the naming-table is ever accessed other than (1) in order to fetch a decl, (2) for functionality which goes through naming-table in hh_server but goes through hh_decl's decl store in the re-architecture.

This diff represents my first two discoveries... In the hot path, we still need
* `get_type_kind` -- to know whether a type represents a typedef or a class, since the two get typechecked quite differently. Hh_server retrieves this information from the naming-table's FLAGS column, but we can happily instead just retrieve it from the decl we get back from hh_decl. That's what I did in this diff,
* `get_typedef_path` -- because for opaque typedefs, we're only allowed to know the definition of the typedef if we're in the same file as the typedef. The existing implementation depends upon the decl having position-full decls. In this diff I called this out.

Reviewed By: CatherineGasnier

Differential Revision: D41294491

fbshipit-source-id: a5c916f1d8960fd60dc49c481c2296d99852ea04
hphp/hack/src/providers/naming_provider.ml
hphp/hack/src/stubs/decl_service_client.ml