[libcxx-commits] [PATCH] D75954: Cache uwnind frame headers as they are found.

[Jorge Gorbe Moya via Phabricator via libcxx-commits]

jgorbe added inline comments.


================
Comment at: libunwind/src/FrameHeaderCache.hpp:45
+
+  CacheEntry Entries[kCacheEntryCount];
+  CacheEntry *MostRecentlyUsed;
----------------
Do we need any synchronization here if there are multiple threads?


================
Comment at: libunwind/src/FrameHeaderCache.hpp:121
+    if (Current != nullptr) {
+      Current = Unused;
+      Unused = Unused->Next;
----------------
This line is redundant, Current has just been initialized to Unused. Did you want to explicitly set Current on each branch? If so, maybe something like this would be clearer?

```
CacheEntry *Current = nullptr;

if (Unused != nullptr) {
  Current = Unused;
  Unused = Unused->Next;
} else {
  Current = MostRecentlyUsed;
  // ... etc ...
}
```

in any case, I think changing the condition to `Unused != nullptr` would be a win, because at least to me it reads more easily as "it there are unused entries...".


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75954