<div dir="ltr">#2 is preferred.  When you do it, make sure to add lldb-commits in the subscribers field.  You're also free to add specific people to the reviewers field, but the mailing list specifically has to be on subscribers.  Unfortunately, due to what I assume are bugs in Phabricator, if you don't get the subscribers right the first time, you have to delete and re-create the revision.  Adding it as part of an update will not cause emails to start being sent to the list.</div><br><div class="gmail_quote"><div dir="ltr">On Thu, Apr 20, 2017 at 11:01 AM Scott Smith <<a href="mailto:scott.smith@purestorage.com">scott.smith@purestorage.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div><div><div>What's the preferred way to post changes?  In the past I tried emailing here but it was pointed out I should send to lldb-commit instead.  But, there's also phabricator for web-based code reviews.<br><br></div>So,<br><br></div>1. just email lldb-commits?<br></div>2. post on <a href="http://reviews.llvm.org/" target="_blank">http://reviews.llvm.org/</a>?<br></div><div class="gmail_extra"><br><div class="gmail_quote">On Thu, Apr 20, 2017 at 3:16 AM, Pavel Labath <span dir="ltr"><<a href="mailto:labath@google.com" target="_blank">labath@google.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">Thank you very much for tracking this down.<div><br></div><div>+1 for making UniqueCStringMap speak ConstString -- i think it just makes sense given that it already has "unique" in the name.</div><div><br></div><div>ConstString already has a GetStringRef accessor. Also adding a conversion operator may be a good idea, although it probably won't help in all situations (you'll still have to write StringRef(X).drop_front() etc. if you want to do stringref operations on the string)</div><span class="m_4690702394290843463HOEnZb"><font color="#888888"><div><br></div><div>pl</div></font></span></div><div class="m_4690702394290843463HOEnZb"><div class="m_4690702394290843463h5"><div class="gmail_extra"><br><div class="gmail_quote">On 20 April 2017 at 01:46, Zachary Turner <span dir="ltr"><<a href="mailto:zturner@google.com" target="_blank">zturner@google.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">It doesn't entirely defeat the purpose, it's just not *as good* as making the interfaces take ConstStrings.  StringRef already has a lot of safety and usability improvements over raw char pointers, and those improvements don't disappear just because you aren't using ConstString.  Although I agree that if you can make it work where the interface only accepts and returns ConstStrings, and make conversion from ConstString to StringRef more seamless, that would be an even better improvement.</div><div class="m_4690702394290843463m_3326392624913773708HOEnZb"><div class="m_4690702394290843463m_3326392624913773708h5"><br><div class="gmail_quote"><div dir="ltr">On Wed, Apr 19, 2017 at 5:33 PM Scott Smith <<a href="mailto:scott.smith@purestorage.com" target="_blank">scott.smith@purestorage.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div>If I just assume the pointers are from ConstString, then doesn't that defeat the purpose of making the interface safer?  Why not use an actual ConstString and provide conversion operators from ConstString to StringRef?  Seems we should be able to rely on the type system to get us safety and performance.<br><br></div>I'll try putting something together tomorrow.<br></div><div class="gmail_extra"><br><div class="gmail_quote">On Wed, Apr 19, 2017 at 4:48 PM, Zachary Turner <span dir="ltr"><<a href="mailto:zturner@google.com" target="_blank">zturner@google.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">The change was made to make the interface safer and allow propagation of StringRef through other layers.  The previous code was already taking a const char *, and so it was working under the assumption that the const char* passed in came from a ConstString.  As such, continuing to make that same assumption seems completely reasonable.<div><br></div><div>So perhaps you can just change the operator to compare the pointers, as was being done before.</div></div><div class="m_4690702394290843463m_3326392624913773708m_-244198736017720605m_2566633864656276268HOEnZb"><div class="m_4690702394290843463m_3326392624913773708m_-244198736017720605m_2566633864656276268h5"><br><div class="gmail_quote"><div dir="ltr">On Wed, Apr 19, 2017 at 4:24 PM Scott Smith <<a href="mailto:scott.smith@purestorage.com" target="_blank">scott.smith@purestorage.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div><div><div>It looks like it was this change:<br><br>commit 45fb8d00309586c3f7027f66f9f8a0b56bf1cc4a<br>Author: Zachary Turner <<a href="mailto:zturner@google.com" target="_blank">zturner@google.com</a>><br>Date:   Thu Oct 6 21:22:44 2016 +0000<br><br>    Convert UniqueCStringMap to use StringRef.<br>    <br>    git-svn-id: <a href="https://llvm.org/svn/llvm-project/lldb/trunk@283494" target="_blank">https://llvm.org/svn/llvm-project/lldb/trunk@283494</a> 91177308-0d34-0410-b5e6-96231b3b80d8<br><br><br></div>I'm guessing it's because the old code assumed const string, which meant that uniqueness comparisons could be done by simply comparing the pointer.  Now it needs to use an actual string comparison routine.  This code:<br><br>     bool operator<(const Entry &rhs) const { return cstring < rhs.cstring; }<br><br></div>didn't actually change in the revision, but cstring went from 'const char *' to 'StringRef'.  If you know for sure that all the StringRefs come from ConstString, then it'd be easy enough to change the comparison, but I don't know how you guarantee that.<br><br></div><div>I assume the change was made to allow proper memory cleanup when the symbols are discarded?<br></div></div><div class="gmail_extra"><br><div class="gmail_quote">On Thu, Apr 13, 2017 at 5:37 AM, Pavel Labath <span dir="ltr"><<a href="mailto:labath@google.com" target="_blank">labath@google.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">Bisecting the performance regression would be extremely valuable. If you want to do that, it would be very appreciated.</div><div class="m_4690702394290843463m_3326392624913773708m_-244198736017720605m_2566633864656276268m_2578618391137199736m_3410255774455019154HOEnZb"><div class="m_4690702394290843463m_3326392624913773708m_-244198736017720605m_2566633864656276268m_2578618391137199736m_3410255774455019154h5"><div class="gmail_extra"><br><div class="gmail_quote">On 12 April 2017 at 20:39, Scott Smith via lldb-dev <span dir="ltr"><<a href="mailto:lldb-dev@lists.llvm.org" target="_blank">lldb-dev@lists.llvm.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div><div>For my app I think it's largely parsing debug symbols tables for shared libraries.  My main performance improvement was to increase the parallelism of parsing that information.<br><br></div>Funny, gdb/gold has a similar accelerator table (created when you link with -gdb-index).  I assume lldb doesn't know how to parse it.<br><br></div>I'll work on bisecting the change.<br></div><div class="m_4690702394290843463m_3326392624913773708m_-244198736017720605m_2566633864656276268m_2578618391137199736m_3410255774455019154m_4823644569432872985HOEnZb"><div class="m_4690702394290843463m_3326392624913773708m_-244198736017720605m_2566633864656276268m_2578618391137199736m_3410255774455019154m_4823644569432872985h5"><div class="gmail_extra"><br><div class="gmail_quote">On Wed, Apr 12, 2017 at 12:26 PM, Jason Molenda <span dir="ltr"><<a href="mailto:jason@molenda.com" target="_blank">jason@molenda.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">I don't know exactly when the 3.9 / 4.0 branches were cut, and what was done between those two points, but in general we don't expect/want to see performance regressions like that.  I'm more familiar with the perf characteristics on macos, Linux is different in some important regards, so I can only speak in general terms here.<br>
<br>
In your example, you're measuring three things, assuming you have debug information for MY_PROGRAM.  The first is "Do the initial read of the main binary and its debug information".  The second is "Find all symbol names 'main'".  The third is "Scan a newly loaded solib's symbols" (assuming you don't have debug information from solibs from /usr/lib etc).  Technically there's some additional stuff here -- launching the process, detecting solibs as they're loaded, looking up the symbol context when we hit the breakpoint, backtracing a frame or two, etc, but that stuff is rarely where you'll see perf issues on a local debug session.<br>
<br>
Which of these is likely to be important will depend on your MY_PROGRAM.  If you have a 'int main(){}', it's not going to be dwarf parsing.  If your binary only pulls in three solib's by the time it is running, it's not going to be new module scanning. A popular place to spend startup time is in C++ name demangling if you have a lot of solibs with C++ symbols.<br>
<br>
<br>
On Darwin systems, we have a nonstandard accelerator table in our DWARF emitted by clang that lldb reads.  The "apple_types", "apple_names" etc tables.  So when we need to find a symbol named "main", for Modules that have a SymbolFile, we can look in the accelerator table.  If that SymbolFile has a 'main', the accelerator table gives us a reference into the DWARF for the definition, and we can consume the DWARF lazily.  We should never need to do a full scan over the DWARF, that's considered a failure.<br>
<br>
(in fact, I'm working on a branch of the <a href="http://llvm.org" rel="noreferrer" target="_blank">llvm.org</a> sources from mid-October and I suspect Darwin lldb is often consuming a LOT more dwarf than it should be when I'm debugging, I need to figure out what is causing that, it's a big problem.)<br>
<br>
<br>
In general, I've been wanting to add a new "perf counters" infrastructure & testsuite to lldb, but haven't had time.  One thing I work on a lot is debugging over a bluetooth connection; it turns out that BT is very slow, and any extra packets we send between lldb and debugserver are very costly.  The communication is so fast over a local host, or over a usb cable, that it's easy for regressions to sneak in without anyone noticing.  So the original idea was hey, we can have something that counts packets for distinct operations.  Like, this "next" command should take no more than 40 packets, that kind of thing.  And it could be expanded -- "b main should fully parse the DWARF for only 1 symbol", or "p *this should only look up 5 types", etc.<br>
<div><div class="m_4690702394290843463m_3326392624913773708m_-244198736017720605m_2566633864656276268m_2578618391137199736m_3410255774455019154m_4823644569432872985m_4317261448361122419h5"><br>
<br>
<br>
<br>
> On Apr 12, 2017, at 11:26 AM, Scott Smith via lldb-dev <<a href="mailto:lldb-dev@lists.llvm.org" target="_blank">lldb-dev@lists.llvm.org</a>> wrote:<br>
><br>
> I worked on some performance improvements for lldb 3.9, and was about to forward port them so I can submit them for inclusion, but I realized there has been a major performance drop from 3.9 to 4.0.  I am using the official builds on an Ubuntu 16.04 machine with 16 cores / 32 hyperthreads.<br>
><br>
> Running: time lldb-4.0 -b -o 'b main' -o 'run' MY_PROGRAM > /dev/null<br>
><br>
> With 3.9, I get:<br>
> real    0m31.782s<br>
> user    0m50.024s<br>
> sys    0m4.348s<br>
><br>
> With 4.0, I get:<br>
> real    0m51.652s<br>
> user    1m19.780s<br>
> sys    0m10.388s<br>
><br>
> (with my changes + 3.9, I got real down to 4.8 seconds!  But I'm not convinced you'll like all the changes.)<br>
><br>
> Is this expected?  I get roughly the same results when compiling llvm+lldb from source.<br>
><br>
> I guess I can spend some time trying to bisect what happened.  5.0 looks to be another 8% slower.<br>
><br>
</div></div>> _______________________________________________<br>
> lldb-dev mailing list<br>
> <a href="mailto:lldb-dev@lists.llvm.org" target="_blank">lldb-dev@lists.llvm.org</a><br>
> <a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev</a><br>
<br>
</blockquote></div><br></div>
</div></div><br>_______________________________________________<br>
lldb-dev mailing list<br>
<a href="mailto:lldb-dev@lists.llvm.org" target="_blank">lldb-dev@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-dev</a><br>
<br></blockquote></div><br></div>
</div></div></blockquote></div><br></div>
</blockquote></div>
</div></div></blockquote></div><br></div>
</blockquote></div>
</div></div></blockquote></div><br></div>
</div></div></blockquote></div><br></div>
</blockquote></div>