[PATCH] D115714: [Debugify] Limit number of processed instructions for original mode

[Stephen Tozer via Phabricator via llvm-commits]

StephenTozer added a comment.

Performance seems like a serious issue when working with large projects here, but I have some questions/thoughts about this approach:

Is there any good reason to have an option to set the limit to zero? Unless I'm missing something, that would be equivalent to just not running debugify at all, which seems like a redundant option to have when debugify is an optional flag to begin with.

More generally, it's probably good to have an optional limit for OriginalDIMode, but I think it would be better if the limit could be passed as an integer instead of an enum. While 10000 may be a good default value for the builds you mentioned in the description, this is only with respect to your hardware setup and time constraints, and even then may be different for other builds (or even different optimization pipelines). Other users may have a higher or lower limit on the number of instructions they can afford to have processed by debugify, and should be able to specify this exactly via command line.

Also this is just my opinion and so I'd like to see what other reviewers think, but personally I think it would be best if the default setting was unlimited (so the limit is purely optional). When we're talking about metric-gathering, I think it's best to have the program do exactly what it says on the tin/what the user would expect it to do, and silently skipping instructions unless you pass an additional flag seems like potentially harmful unexpected behaviour. The documentation should be updated to include this option so that if a user does encounter performance issues then they will know that the option is there, but they will never unknowingly get incomplete results because they didn't know about this flag.


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

https://reviews.llvm.org/D115714