[Lldb-commits] [PATCH] D123020: increase timeouts if running on valgrind

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

> BTW, does it make sense to get even things like this reviewed, or is it ok if I push these directly if I'm reasonably certain I know what I'm doing? I feel like I'm spamming you by now.

Generally, I would say yes. I'm not even sure that some of your other patches should have been submitted without a pre-commit review (*all* llvm patches are expected to be reviewed).

In principle this idea seems fine, but the implementation is somewhat odd. The caller of ScopedTimeout is passing a specific timeout value, but then the class sets the timeout to something completely different. And it means you're not increasing the timeout for regular (fast) packets, so they still get to run with the default timeout of 1 second.

I would say that the entire ScopedTimeout class should be multiplier-based. Instead of passing a fix value, the user would say "I know this packed is slow, so I want to use X-times the usual timeout value". Then, we could support valgrind (and various *SANs) by just setting the initial timeout value to a reasonably high value, and the multipliers would take care of the rest.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123020