[PATCH] D109632: [clang] de-duplicate methods from AST files

[Volodymyr Sapsai via Phabricator via cfe-commits]

vsapsai added a comment.

In D109632#3013620 <https://reviews.llvm.org/D109632#3013620>, @dexonsmith wrote:

> In D109632#3013523 <https://reviews.llvm.org/D109632#3013523>, @vsapsai wrote:
>
>> 2. Serialize only methods owned by the current module (and change `ReadMethodPoolVisitor` appropriately).
>
> Would that require visiting all in-memory modules every time there's a global method pool lookup?
>
> If so, that sounds expensive... and similar to the problem that had to be solved to make identifier lookup fast. Maybe the same approach can/should be used here?
>
> Or if not, can you clarify for me?

We use `ReadMethodPoolVisitor` only from `ASTReader::ReadMethodPool`. I don't know all the cases when it is called but it is called at least on encountering `@end` to handle methods declared in an interface. With the proposed change the behavior is the following:

- the first time we encounter a selector, we walk the entire transitive closure of dependencies
- when `ReadMethodPool` is called again for the same selector, we check only immediate dependencies but exit early and don't drill into transitive dependencies due to

  // If we've already searched this module file, skip it now.
  if (M.Generation <= PriorGeneration)
    return true;

- when we add a new dependency and trigger `ReadMethodPool` again, we visit only the added dependency and its unvisited transitive dependencies. We exit early for already visited modules.

So from the module visiting perspective there are no regressions. I'm not super happy with repeated early returns for immediate dependencies but we were doing it already. This behavior seems to be orthogonal and we can address it separately if needed.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D109632/new/

https://reviews.llvm.org/D109632