[Lldb-commits] [PATCH] D106584: [lldb] Improve checking of file cache read eligibility for mach-O

Sat Jul 24 01:30:25 PDT 2021

jasonmolenda added a comment.

In D106584#2902200 <https://reviews.llvm.org/D106584#2902200>, @jasonmolenda wrote:

> 

> In D106584#2900239 <https://reviews.llvm.org/D106584#2900239>, @augusto2112 wrote:
>
>> Hi Jason,
>>
>> At least that we can keep then :) Do you think it's should be a crash, or maybe a log? Wasn't sure what was best.
>
> I think an assert is good, there's no scenario where this should get hit -- if it does, then we really have a bug in using the filecache data and we need to halt immediately in CI or on an lldb developer's desktop.  I'm a little less sure about thing this on for DEBUG builds all the time because it does change lldb's perf behavior a lot, specifically adding lots of packet traffic, but I think we should give it a try and see if it turns out to be too much of a change.

If the perf behavior of lldb changes too much, we could put the "also read from live memory & memcmp against file cache when we use filecache" behind a setting, and set it for testsuite runs, so we get this used in CI and desktop testruns.  The extra packet traffic in those cases is going to be less important.  Maybe we start with the easy "always enable on DEBUG builds" and fall back to a setting when it turns out to be too much, unless other people have an opinion.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106584