clayborg added a comment.
In https://reviews.llvm.org/D31451#987537, @labath wrote:
> This code is processing demangled names. Since you say (I could not get my
> demangler to process it either) the symbol demangles to a multi-megabyte
> name, we can probably make the cutoff even longer then 10
labath added a comment.
This code is processing demangled names. Since you say (I could not get my
demangler to process it either) the symbol demangles to a multi-megabyte name,
we can probably make the cutoff even longer then 1000 bytes.
OTOH, if we abort demangling of such names in the first
clayborg added a comment.
The funny thing is this is only 981 characters long... Not sure what the right
cutoff would be...
Repository:
rL LLVM
https://reviews.llvm.org/D31451
___
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://list
eugene added a comment.
Greg, this name is amazing. My c++filt refuses to demangle it. We can probably
give up on parsing C++ names if they're longer than 1000 characters or
something.
Repository:
rL LLVM
https://reviews.llvm.org/D31451
___
lld
clayborg added a comment.
This is part of the problem, but not the entire thing.. We had a mangled name:
_ZNK3shk6detail17CallbackPublisherIZNS_5ThrowERKNSt15__exception_ptr13exception_ptrEEUlOT_E_E9SubscribeINS0_9ConcatMapINS0_18CallbackSubscriberIZNS_6GetAllIiNS1_IZZNS_9ConcatMapIZNS_6ConcatIJNS
clayborg added a comment.
Herald added subscribers: llvm-commits, hintonda.
This code is making debugging of large C++ apps so slow it is unusable...
Repository:
rL LLVM
https://reviews.llvm.org/D31451
___
lldb-commits mailing list
lldb-commits@l
Thanks for vigilance. I added a couple more tests in r299729.
On Thu, Apr 6, 2017 at 4:06 PM, Jim Ingham via Phabricator <
revi...@reviews.llvm.org> wrote:
> jingham added a comment.
>
> I'm sorry, I don't have time actually review the code here for
> correctness... But can you make sure that th
jingham added a comment.
I'm sorry, I don't have time actually review the code here for correctness...
But can you make sure that this also rejects a two or three field selector, not
just "selector:" but "selector:otherField:"? That seems sufficiently different
that you might get the one : bu
No problem. You can't test on all platforms, so changes like this are bound to
sometimes introduce regressions. If you can remember to watch the bots for
systems you don't run or test on that makes the turnaround smoother.
Otherwise, if it's a MacOS problem the QE folks here are sure to tell
Sorry about that and thanks for rolling it back. I'll take a look tomorrow.
On Tue, Apr 4, 2017 at 5:22 PM, Jim Ingham wrote:
> This patch is causing a testsuite failure in ObjC breakpoint setting:
>
> functionalities/breakpoint/objc/TestObjCBreakpoints.py
>
> You must be grabbing ObjC names and
This patch is causing a testsuite failure in ObjC breakpoint setting:
functionalities/breakpoint/objc/TestObjCBreakpoints.py
You must be grabbing ObjC names and somehow treating them as C++ names.
I reverted the patch till the test gets fixed. If you can't get your hands on
an ObjC binary to t
This revision was automatically updated to reflect the committed changes.
Closed by commit rL299374: New C++ function name parsing logic (authored by
eugene).
Changed prior to commit:
https://reviews.llvm.org/D31451?vs=93694&id=93910#toc
Repository:
rL LLVM
https://reviews.llvm.org/D31451
labath accepted this revision.
labath added a comment.
This revision is now accepted and ready to land.
Thank you
In https://reviews.llvm.org/D31451#715664, @eugene wrote:
> In https://reviews.llvm.org/D31451#715649, @tberghammer wrote:
>
> > Because of this I think some targeted micro benchmark
eugene added a subscriber: labath.
eugene marked 2 inline comments as done.
eugene added a comment.
In https://reviews.llvm.org/D31451#715649, @tberghammer wrote:
> Because of this I think some targeted micro benchmark will be much more
> useful to measure the performance of this code then an en
eugene updated this revision to Diff 93694.
eugene marked an inline comment as done.
https://reviews.llvm.org/D31451
Files:
source/Plugins/Language/CPlusPlus/CMakeLists.txt
source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.h
source
eugene updated this revision to Diff 93693.
eugene marked 3 inline comments as done.
eugene added a comment.
Addressing review commnets
https://reviews.llvm.org/D31451
Files:
source/Plugins/Language/CPlusPlus/CMakeLists.txt
source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
source/Pl
tberghammer added a comment.
One note on benchmarking: A did some performance profiling on LLDB using a
similar approach to what Pavel suggested and if I remember correctly only ~10%
of the time was spent on C++ name parsing (~15% was C++ demangling, ~50% was
debug_info parsing, rest of them wa
labath added a comment.
In https://reviews.llvm.org/D31451#714850, @eugene wrote:
> I did some micro-benchmarking and on average new parser is ~3 time slower
> than the old one. (new parser - ~200k string/s, old parser - ~700k string/s)
> clang::Lexer appears to be the slowest part of it.
>
>
eugene marked 4 inline comments as done.
eugene added a comment.
> I think we should do some performance measurements to see whether this needs
> more optimising or it's fine as is.
>
> I propose the following benchmark:
>
> bin/lldb bin/clang
>
> make sure clang is statically linked
> br
eugene updated this revision to Diff 93572.
eugene added a comment.
Addressing code-review comments.
Most notable change: MethodName::Parse() tries simple version of name parser,
before invoking full power of CPlusPlusNameParser. It really helps with the
perf.
https://reviews.llvm.org/D31451
labath added a comment.
I cant help but feel that this could have been done in a simpler way, but then
again, some of the cases you have dug up are quite tricky.
I think we should do some performance measurements to see whether this needs
more optimising or it's fine as is.
I propose the follo
eugene updated this revision to Diff 93438.
eugene added reviewers: labath, zturner, jingham.
eugene changed the visibility from "eugene (Eugene Zemtsov)" to "Public (No
Login Required)".
eugene changed the edit policy from "eugene (Eugene Zemtsov)" to "All Users".
eugene added a subscriber: lldb-
22 matches
Mail list logo
