[llvm-commits] [lld] r150547 - in /lld/trunk: include/lld/Core/SymbolTable.h lib/Core/SymbolTable.cpp

[Chris Lattner]

On Feb 14, 2012, at 4:50 PM, Nick Kledzik wrote:
> URL: http://llvm.org/viewvc/llvm-project?rev=150547&view=rev
> Log:
> use llvm::DenseMap instead of std::map

Great.  One funny thing sticks out at me though:

> +  struct StringRefMappingInfo {
> +    static llvm::StringRef getEmptyKey() { return llvm::StringRef(); }
> +    static llvm::StringRef getTombstoneKey() { return llvm::StringRef(" ", 0); }
> +    static unsigned getHashValue(llvm::StringRef const val) {
> +                                               return llvm::HashString(val); }
> +    static bool isEqual(llvm::StringRef const lhs, 
> +                        llvm::StringRef const rhs) { return lhs.equals(rhs); }
> +  };
> +	typedef llvm::DenseMap<llvm::StringRef, const Atom *, 
> +                                           StringRefMappingInfo> NameToAtom;

Why a DenseMap of StringRef?  Are the strings that you are hashing nul terminated?  If so, you can save some space in the DenseMap by just keeping the char* as a key.

Secondarily, if the ownership of your strings is not known (so the map needs to copy them) then it makes sense to use StringMap.  I assume that you have permanent pointers  around in mapped files, so you don't have to worry about this though.

-Chris