[Lldb-commits] [PATCH] D153648: [lldb] Remove broken shared cache duplicate class handling

[Dave Lee via Phabricator via lldb-commits]

kastiglione added a comment.

In D153648#4444898 <https://reviews.llvm.org/D153648#4444898>, @jingham wrote:

> and that the one that won will indeed be in that list but will not be marked as a duplicate.

All of the duplicate classes are in the duplicate list, not a single class that won. For example, a duplicated class, say `ABCSomeController`, will be in the duplicate list 2 or more times. The result is that there's redundant hashing, since each of the duplicate names occurs 2 or more times. Which is maybe fine, or maybe causes a perf penalty. Another result is that `ObjCLanguageRuntime::m_hash_to_isa_map` (a `multimap`) ends up storing every duplicate, however the readers of `m_hash_to_isa_map` always take the first match found (see `ObjCLanguageRuntime::GetDescriptorIterator`). In other words, the redundant classes are never accessed, and the one lldb does return could differ from the class the objc runtime has picked. A "fix" to the surface level bugs identified here has the result of exposing a bunch of non-determinism.

My thought process was: "This is broken to the point that duplicate classes are currently being dropped altogether, and nobody has identified this as a problem. Maybe delete the code until we can figure out a proper fix."

I am fine with the "fix", but as you can see I am very skeptical in it being helpful.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153648