[PATCH] D16597: Query the StringMap only once when creating MDString (NFC)

[David Blaikie via llvm-commits]

On Tue, Jan 26, 2016 at 12:34 PM, Mehdi AMINI via llvm-commits <
llvm-commits at lists.llvm.org> wrote:

> joker.eph created this revision.
> joker.eph added a reviewer: dexonsmith.
> joker.eph added a subscriber: llvm-commits.
>
> Loading IR with debug info improves MDString::get() from 19ms to 10ms.
>
> http://reviews.llvm.org/D16597
>
> Files:
>   include/llvm/IR/Metadata.h
>   lib/IR/Metadata.cpp
>
> Index: lib/IR/Metadata.cpp
> ===================================================================
> --- lib/IR/Metadata.cpp
> +++ lib/IR/Metadata.cpp
> @@ -399,17 +399,12 @@
>
>  MDString *MDString::get(LLVMContext &Context, StringRef Str) {
>    auto &Store = Context.pImpl->MDStringCache;
> -  auto I = Store.find(Str);
> -  if (I != Store.end())
> -    return &I->second;
> -
> -  auto *Entry =
> -      StringMapEntry<MDString>::Create(Str, Store.getAllocator(),
> MDString());
> -  bool WasInserted = Store.insert(Entry);
> -  (void)WasInserted;
> -  assert(WasInserted && "Expected entry to be inserted");
> -  Entry->second.Entry = Entry;
> -  return &Entry->second;
> +  auto I = Store.insert(std::make_pair(Str, MDString()));
> +  auto &MapEntry = I.first->getValue();
> +  if (!I.second)
> +    return &MapEntry;
> +  MapEntry.Entry = &*I.first;
> +  return &MapEntry;
>  }
>
>  StringRef MDString::getString() const {
> Index: include/llvm/IR/Metadata.h
> ===================================================================
> --- include/llvm/IR/Metadata.h
> +++ include/llvm/IR/Metadata.h
> @@ -599,9 +599,9 @@
>
>    StringMapEntry<MDString> *Entry;
>    MDString() : Metadata(MDStringKind, Uniqued), Entry(nullptr) {}
> -  MDString(MDString &&) : Metadata(MDStringKind, Uniqued) {}
>

Well that was weird/scary/bogus ^


>
>  public:
> +  MDString(MDString &&R) : Metadata(MDStringKind, Uniqued),
> Entry(R.Entry) {}
>

Could we just remove this, and rely on the implicit move/copy ctors? ^ I
guess maybe the base copy/move ctor doesn't do the right thing (since it's
not being used here)

Should we just make this the copy ctor? It doesn't seme to really be moving
anything.


>    static MDString *get(LLVMContext &Context, StringRef Str);
>    static MDString *get(LLVMContext &Context, const char *Str) {
>      return get(Context, Str ? StringRef(Str) : StringRef());
>
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160126/d00580ae/attachment.html>