[Lldb-commits] [lldb] r281273 - This is the main part of a change to add breakpoint save and restore to lldb.

[Zachary Turner via lldb-commits]

>
>
> Immediately, very little.  A small amount of performance, since comparing
> StringRefs is faster than comparing null terminated stings, since the
> length is stored with the string it can use memcmp instead of strcmp.
>
> From a big picture perspective, quite a lot IMO.  StringRef has numerous
> methods which are extremely useful and not easy to reproduce correctly.
> Also, If you do the comparison many times, you only pay for the length
> computation once.
>

Also, this email is so long that it's truncating in my email reader.  So
I'm not sure if the second part of my email went through.  It's possible
your reader is better than mine at handling large emails, but I'm copying
the answer to your second question below just in case you also didn't see
it.

Thanks for the comments.
>
>   I don't see the benefit of using StringRef's to return all the key
names.  I'm generally only ever going to pass them to the StructuredData
API's, which makes them into StringRef's transparently on the way in.  How
would building StringRefs help here?

>   You also suggested changing a bunch of BreakpointOption API to return
StringRef's.  OTOH this CL just mechanically changed from m_options to
m_options_up, so changing the API was not part of the patches intent.  OTOH
most of these options (condition, thread name, etc) can take and return
NULL strings.  I didn't think StringRef's handled null strings well, was I
wrong about that?  And again, what would I gain by making these
StringRef's?  I'm just going to stash them away in some storage the object
owns, and I'm not going to ever share the storage with the caller.  So
least common denominator (const char *) seems a better choice here.  If the
caller wants a StringRef they are welcome to make one.

Right, but making the StringRef incurs a length computation.  That's not
something you want to do over and over.  It's guaranteed someone somewhere
is going to compute the length, so it's better to do it once upfront, and
allow everyone else to never have to do it again.

On the other hand, using a StringRef gives you many advantages.  Unless you
know every possible way in which these strings will ever be used, who knows
what someone might want to do with it?  What if someone wants to take one
of these strings, check if some other string starts with it, and chop it
off if so?  You could either write:

if (strncmp(GetName(), str.c_str(), strlen(GetName()) == 0)
    str2 = str.substr(strlen(GetName()));

which there's a good chance will be either wrong or suboptimal, or you
could write:

str.consume_front(GetName());

which is both easier to understand, obviously correct, and optimal from a
performance standpoint.

const char* should almost never be used for anything unless you absolutely
must interface with a C-api for which there is no good equivalent on
StringRef (very rare).

Since we currently use const char* in many places, this sometimes makies
interfacing difficult, but as more common classes move to StringRef, this
will go away and almost all code will become faster and easier to
understand.

You are right that StringRefs don't handle null strings, but they do handle
empty strings, and it's not common that empty and null need to be treated
as distinct values.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/lldb-commits/attachments/20160913/5fb61e25/attachment.html>