labath added inline comments.
Comment at: source/Utility/ConstString.cpp:49
+ // pointer, we don't need the lock.
const StringPoolEntryType = GetStringMapEntryFromKeyData(ccstr);
return entry.getKey().size();
scott.smith wrote:
> labath
This revision was automatically updated to reflect the committed changes.
Closed by commit rL301642: Remove lock from ConstString::GetLength (authored by
labath).
Changed prior to commit:
https://reviews.llvm.org/D32306?vs=96841=97083#toc
Repository:
rL LLVM
https://reviews.llvm.org/D32306
scott.smith marked 3 inline comments as done.
scott.smith added inline comments.
Comment at: source/Utility/ConstString.cpp:49
+ // pointer, we don't need the lock.
const StringPoolEntryType = GetStringMapEntryFromKeyData(ccstr);
return entry.getKey().size();
Other alternatives though include adding a function to StringMap which
gives us what we need, and having this function do the pointer hacking, or
simply calling strlen.
On Wed, Apr 26, 2017 at 11:03 AM Scott Smith via Phabricator <
revi...@reviews.llvm.org> wrote:
> scott.smith added inline
scott.smith added inline comments.
Comment at: source/Utility/ConstString.cpp:49
+ // pointer, we don't need the lock.
const StringPoolEntryType = GetStringMapEntryFromKeyData(ccstr);
return entry.getKey().size();
zturner wrote:
> Why do we
labath added a comment.
In https://reviews.llvm.org/D32306#736115, @scott.smith wrote:
> 10.2% reduction in # of instructions executed, 9.1% reduction in # of cycles,
> as measured by 'perf stat' in single threaded mode (I disabled TaskPool in
> order to get more repeatable results).
That is
labath added inline comments.
Comment at: source/Utility/ConstString.cpp:49
+ // pointer, we don't need the lock.
const StringPoolEntryType = GetStringMapEntryFromKeyData(ccstr);
return entry.getKey().size();
zturner wrote:
> Why do we even
zturner added inline comments.
Comment at: source/Utility/ConstString.cpp:49
+ // pointer, we don't need the lock.
const StringPoolEntryType = GetStringMapEntryFromKeyData(ccstr);
return entry.getKey().size();
Why do we even have this
scott.smith added a comment.
In https://reviews.llvm.org/D32306#733316, @labath wrote:
> Looks good, thank you.
>
> Out of curiosity, have you observed any performance improvements resulting
> from this?
10.2% reduction in # of instructions executed, 9.1% reduction in # of cycles,
as
labath accepted this revision.
labath added a comment.
This revision is now accepted and ready to land.
Looks good, thank you.
Out of curiosity, have you observed any performance improvements resulting from
this?
Repository:
rL LLVM
https://reviews.llvm.org/D32306
scott.smith created this revision.
ConstStrings are immutable, so there is no need to grab even a reader lock in
order to read the length field.
Repository:
rL LLVM
https://reviews.llvm.org/D32306
Files:
source/Utility/ConstString.cpp
Index: source/Utility/ConstString.cpp
11 matches
Mail list logo
