[llvm] r270449 - llvm-dwp: Add an abstraction for the DWP string pool

[David Blaikie via llvm-commits]

On Tue, May 24, 2016 at 10:46 AM, Rafael Espíndola <
rafael.espindola at gmail.com> wrote:

> Nah, it is fine. Just surprised that computing strlen once didn't make
> it worth it.
>

I've not really been measuring runtime, since we're hard up against memory
limits for some large files - so I've just been optimizing for memory usage
at the moment. Seeing how far down we can get it (turns out: not enough to
matter in the case we care about - hence I'm just tidying up a few loose
ends/trying to make the code a bit neater before shelving it for now (no
doubt will come back to it within the next year or two))

So it's possible that the extra strlen computation does add significant
runtime compared to caching it - but I've not checked that.

- Dave


>
> Thanks,
> Rafael
>
> On 24 May 2016 at 11:39, David Blaikie <dblaikie at gmail.com> wrote:
> >
> >
> > On Tue, May 24, 2016 at 4:54 AM, Rafael Espíndola
> > <rafael.espindola at gmail.com> wrote:
> >>
> >> On 23 May 2016 at 12:32, David Blaikie via llvm-commits
> >> <llvm-commits at lists.llvm.org> wrote:
> >> > Author: dblaikie
> >> > Date: Mon May 23 11:32:11 2016
> >> > New Revision: 270449
> >> >
> >> > URL: http://llvm.org/viewvc/llvm-project?rev=270449&view=rev
> >> > Log:
> >> > llvm-dwp: Add an abstraction for the DWP string pool
> >> >
> >> > Also reference strings in the memory mapped file, reduces memory usage
> >> > on a large test case by 18.5%.
> >> >
> >> > Added:
> >> >     llvm/trunk/tools/llvm-dwp/DWPStringPool.h
> >> > Modified:
> >> >     llvm/trunk/tools/llvm-dwp/DWPError.h
> >> >     llvm/trunk/tools/llvm-dwp/llvm-dwp.cpp
> >> >
> >> > Modified: llvm/trunk/tools/llvm-dwp/DWPError.h
> >> > URL:
> >> >
> http://llvm.org/viewvc/llvm-project/llvm/trunk/tools/llvm-dwp/DWPError.h?rev=270449&r1=270448&r2=270449&view=diff
> >> >
> >> >
> ==============================================================================
> >> > --- llvm/trunk/tools/llvm-dwp/DWPError.h (original)
> >> > +++ llvm/trunk/tools/llvm-dwp/DWPError.h Mon May 23 11:32:11 2016
> >> > @@ -1,6 +1,10 @@
> >> > +#ifndef TOOLS_LLVM_DWP_DWPERROR
> >> > +#define TOOLS_LLVM_DWP_DWPERROR
> >> > +
> >> >  #include "llvm/Support/Error.h"
> >> >  #include "llvm/Support/ErrorHandling.h"
> >> >  #include <string>
> >> > +
> >> >  namespace llvm {
> >> >  class DWPError : public ErrorInfo<DWPError> {
> >> >  public:
> >> > @@ -15,3 +19,5 @@ private:
> >> >    std::string Info;
> >> >  };
> >> >  }
> >> > +
> >> > +#endif
> >> >
> >> > Added: llvm/trunk/tools/llvm-dwp/DWPStringPool.h
> >> > URL:
> >> >
> http://llvm.org/viewvc/llvm-project/llvm/trunk/tools/llvm-dwp/DWPStringPool.h?rev=270449&view=auto
> >> >
> >> >
> ==============================================================================
> >> > --- llvm/trunk/tools/llvm-dwp/DWPStringPool.h (added)
> >> > +++ llvm/trunk/tools/llvm-dwp/DWPStringPool.h Mon May 23 11:32:11 2016
> >> > @@ -0,0 +1,54 @@
> >> > +#ifndef TOOLS_LLVM_DWP_DWPSTRINGPOOL
> >> > +#define TOOLS_LLVM_DWP_DWPSTRINGPOOL
> >> > +
> >> > +#include "llvm/ADT/DenseMap.h"
> >> > +#include "llvm/MC/MCSection.h"
> >> > +#include "llvm/MC/MCStreamer.h"
> >> > +#include <cassert>
> >> > +
> >> > +class DWPStringPool {
> >> > +
> >> > +  struct CStrDenseMapInfo {
> >> > +    static inline const char *getEmptyKey() {
> >> > +      return reinterpret_cast<const char
> >> > *>(~static_cast<uintptr_t>(0));
> >> > +    }
> >> > +    static inline const char *getTombstoneKey() {
> >> > +      return reinterpret_cast<const char
> >> > *>(~static_cast<uintptr_t>(1));
> >> > +    }
> >> > +    static unsigned getHashValue(const char *Val) {
> >> > +      assert(Val != getEmptyKey() && "Cannot hash the empty key!");
> >> > +      assert(Val != getTombstoneKey() && "Cannot hash the tombstone
> >> > key!");
> >> > +      return (unsigned)hash_value(StringRef(Val));
> >> > +    }
> >> > +    static bool isEqual(const char *LHS, const char *RHS) {
> >> > +      if (RHS == getEmptyKey())
> >> > +        return LHS == getEmptyKey();
> >> > +      if (RHS == getTombstoneKey())
> >> > +        return LHS == getTombstoneKey();
> >> > +      return strcmp(LHS, RHS) == 0;
> >> > +    }
> >> > +  };
> >> > +
> >> > +  MCStreamer &Out;
> >> > +  MCSection *Sec;
> >> > +  DenseMap<const char *, uint32_t, CStrDenseMapInfo> Pool;
> >> > +  uint32_t Offset = 0;
> >>
> >> Can't you use a DenseMap of StringRef?
> >
> >
> > StringRef has a pointer and length - storing only a pointer is noticeably
> > smaller due to there being quite so many of these. I can double back &
> check
> > the numbers if you're curious.
> >
> > - Dave
> >
> >>
> >>
> >> Cheers,
> >> Rafael
> >
> >
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160524/d5ecb480/attachment.html>