jingham added a comment.
I'm not much surprised by that, but thanks for the experiments. As you say,
that wasn't the primary purpose of this patch.
Repository:
rL LLVM
https://reviews.llvm.org/D51453
___
lldb-commits mailing list
aprantl marked an inline comment as done.
aprantl added a comment.
FYI: I tried to benchmark this using `break set -A -p begin` and similar
things, but in all my experiments the variation between test runs was much
larger than any difference with or without my patch. The filtering of the
This revision was automatically updated to reflect the committed changes.
Closed by commit rL340994: Refactor BreakpointResolver::SetSCMatchesByLine() to
make it easier to (authored by adrian, committed by ).
Herald added a subscriber: llvm-commits.
Changed prior to commit:
jingham accepted this revision.
jingham added a comment.
This revision is now accepted and ready to land.
Oh, yeah, gotta remember to check the box...
https://reviews.llvm.org/D51453
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
jingham added a comment.
Except for the nit about the comment, this looks fine.
https://reviews.llvm.org/D51453
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
teemperor added a comment.
lldb-bench now has a benchmark called `br-by-regex` that should profile this
code. It should run tonight, so if merge this tomorrow or later we should be
able to see the performance impact of this patch.
https://reviews.llvm.org/D51453
jingham added a comment.
By name breakpoints don't use this function so a function regex wouldn't show
the change. But the source regex does use this filter, so something like:
(lldb) break set -p ;
on a large source file might do it. That's a pretty silly thing to do,
however. The only
aprantl updated this revision to Diff 163198.
aprantl marked 2 inline comments as done.
aprantl added a comment.
Address review feedback.
https://reviews.llvm.org/D51453
Files:
include/lldb/Breakpoint/BreakpointResolver.h
source/Breakpoint/BreakpointResolver.cpp
Index:
aprantl marked an inline comment as done.
aprantl added a comment.
I don't have any numbers to back up the performance claim. My primary
motivation for this patch was to prepare the code for adding extra
functionality; the performance gain was only side-effect. One way to measure it
would be
shafik added a comment.
Very nice! Do we have a way of verifying the performance gain?
Comment at: source/Breakpoint/BreakpointResolver.cpp:183
llvm::StringRef log_ident) {
- Log
aprantl created this revision.
aprantl added a reviewer: jingham.
Herald added a subscriber: mgrang.
Refactor BreakpointResolver::SetSCMatchesByLine() to make it easier to
read/understand/maintain.
As a side-effect, this should also improve the performance by avoiding
lots ofcostly vector
11 matches
Mail list logo
