[clang-tools-extra] r318840 - [FindAllSymbols] Cache regexes, creating them is expensive

Krzysztof Parzyszek via cfe-commits cfe-commits at lists.llvm.org
Thu Nov 23 10:05:21 PST 2017


There has been some problem with std::pair on FreeBSD (due to some ABI 
compatibility issue), but I don't know much about what it was.

Commenting the reserve doesn't help, unfortunately. The same problem is 
now flagged in emplace_back.

-Krzysztof


On 11/23/2017 11:47 AM, Benjamin Kramer wrote:
> That looks like a bug in the standard library. Does removing the call
> to reserve fix it? It's not really necessary, that code isn't
> performance sensitive at all.
> 
> On Thu, Nov 23, 2017 at 6:36 PM, Krzysztof Parzyszek
> <kparzysz at codeaurora.org> wrote:
>> Hi,
>> This broke build on FreeBSD 11:
>>
>> [100%] Building CXX object
>> tools/clang/tools/extra/include-fixer/find-all-symbols/CMakeFiles/findAllSymbols.dir/HeaderMapCollector.cpp.o
>> cd /w/bld/org/tools/clang/tools/extra/include-fixer/find-all-symbols &&
>> /w/c/clang+llvm-5.0.0-x86_64-unknown-freebsd11/bin/clang++
>> -DGTEST_HAS_RTTI=0 -D_DEBUG -D_GNU_SOURCE -D__STDC_CONSTANT_MACROS
>> -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS
>> -I/w/bld/org/tools/clang/tools/extra/include-fixer/find-all-symbols
>> -I/w/src/llvm.org/tools/clang/tools/extra/include-fixer/find-all-symbols
>> -I/w/src/llvm.org/tools/clang/include -I/w/bld/org/tools/clang/include
>> -I/w/bld/org/include -I/w/src/llvm.org/include -isystem /usr/local/include
>> -stdlib=libc++ -fPIC -fvisibility-inlines-hidden -Werror=date-time
>> -Werror=unguarded-availability-new -std=c++11 -Wall -W -Wno-unused-parameter
>> -Wwrite-strings -Wcast-qual -Wmissing-field-initializers -pedantic
>> -Wno-long-long -Wcovered-switch-default -Wnon-virtual-dtor
>> -Wdelete-non-virtual-dtor -Wno-comment -Wstring-conversion
>> -ffunction-sections -fdata-sections -fno-common -Woverloaded-virtual
>> -Wno-nested-anon-types -O3    -UNDEBUG -fno-exceptions -fno-rtti -o
>> CMakeFiles/findAllSymbols.dir/HeaderMapCollector.cpp.o -c
>> /w/src/llvm.org/tools/clang/tools/extra/include-fixer/find-all-symbols/HeaderMapCollector.cpp
>> In file included from
>> /w/src/llvm.org/tools/clang/tools/extra/include-fixer/find-all-symbols/HeaderMapCollector.cpp:10:
>> In file included from
>> /w/src/llvm.org/tools/clang/tools/extra/include-fixer/find-all-symbols/HeaderMapCollector.h:13:
>> In file included from /w/src/llvm.org/include/llvm/ADT/StringMap.h:17:
>> In file included from /w/src/llvm.org/include/llvm/ADT/StringRef.h:13:
>> In file included from /w/src/llvm.org/include/llvm/ADT/STLExtras.h:20:
>> In file included from /w/src/llvm.org/include/llvm/ADT/Optional.h:22:
>> In file included from /w/src/llvm.org/include/llvm/Support/type_traits.h:19:
>> /usr/include/c++/v1/utility:315:11: error: call to deleted constructor of
>> 'llvm::Regex'
>>          : first(__p.first),
>>            ^     ~~~~~~~~~
>> /usr/include/c++/v1/memory:1747:31: note: in instantiation of member
>> function 'std::__1::pair<llvm::Regex, const char *>::pair' requested here
>>              ::new((void*)__p) _Up(_VSTD::forward<_Args>(__args)...);
>>                                ^
>> /usr/include/c++/v1/memory:1658:18: note: in instantiation of function
>> template specialization 'std::__1::allocator<std::__1::pair<llvm::Regex,
>> const char *> >::construct<std::__1::pair<llvm::Regex, const char *>, const
>> std::__1::pair<llvm::Regex, const char *> &>' requested here
>>              {__a.construct(__p, _VSTD::forward<_Args>(__args)...);}
>>                   ^
>> /usr/include/c++/v1/memory:1504:14: note: in instantiation of function
>> template specialization
>> 'std::__1::allocator_traits<std::__1::allocator<std::__1::pair<llvm::Regex,
>> const char *> > >::__construct<std::__1::pair<llvm::Regex, const char *>,
>> const std::__1::pair<llvm::Regex, const char *> &>' requested here
>>              {__construct(__has_construct<allocator_type, _Tp*, _Args...>(),
>>               ^
>> /usr/include/c++/v1/memory:1620:17: note: in instantiation of function
>> template specialization
>> 'std::__1::allocator_traits<std::__1::allocator<std::__1::pair<llvm::Regex,
>> const char *> > >::construct<std::__1::pair<llvm::Regex, const char *>,
>> const std::__1::pair<llvm::Regex, const char *> &>' requested here
>>                  construct(__a, _VSTD::__to_raw_pointer(__end2-1),
>> _VSTD::move_if_noexcept(*--__end1));
>>                  ^
>> /usr/include/c++/v1/vector:892:21: note: in instantiation of function
>> template specialization
>> 'std::__1::allocator_traits<std::__1::allocator<std::__1::pair<llvm::Regex,
>> const char *> > >::__construct_backward<std::__1::pair<llvm::Regex, const
>> char *> *>' requested here
>>      __alloc_traits::__construct_backward(this->__alloc(), this->__begin_,
>> this->__end_, __v.__begin_);
>>                      ^
>> /usr/include/c++/v1/vector:1537:9: note: in instantiation of member function
>> 'std::__1::vector<std::__1::pair<llvm::Regex, const char *>,
>> std::__1::allocator<std::__1::pair<llvm::Regex, const char *> >
>>> ::__swap_out_circular_buffer' requested here
>>          __swap_out_circular_buffer(__v);
>>          ^
>> /w/src/llvm.org/tools/clang/tools/extra/include-fixer/find-all-symbols/HeaderMapCollector.cpp:19:33:
>> note: in instantiation of member function
>> 'std::__1::vector<std::__1::pair<llvm::Regex, const char *>,
>> std::__1::allocator<std::__1::pair<llvm::Regex, const char *> > >::reserve'
>> requested here
>>    this->RegexHeaderMappingTable.reserve(RegexHeaderMappingTable->size());
>>                                  ^
>> /w/src/llvm.org/include/llvm/Support/Regex.h:49:5: note: 'Regex' has been
>> explicitly marked deleted here
>>      Regex(const Regex &) = delete;
>>      ^
>> 1 error generated.
>> *** Error code 1
>>
>>
>> -Krzysztof
>>
>>
>>
>> On 11/22/2017 9:38 AM, Benjamin Kramer via cfe-commits wrote:
>>>
>>> Author: d0k
>>> Date: Wed Nov 22 07:38:23 2017
>>> New Revision: 318840
>>>
>>> URL: http://llvm.org/viewvc/llvm-project?rev=318840&view=rev
>>> Log:
>>> [FindAllSymbols] Cache regexes, creating them is expensive
>>>
>>> This is a bit annoying because LLVM regexes are always mutable to store
>>> errors. Assert that there are never errors and fix broken hardcoded
>>> regexes.
>>>
>>> Modified:
>>>
>>> clang-tools-extra/trunk/include-fixer/find-all-symbols/HeaderMapCollector.cpp
>>>
>>> clang-tools-extra/trunk/include-fixer/find-all-symbols/HeaderMapCollector.h
>>>
>>> clang-tools-extra/trunk/include-fixer/find-all-symbols/STLPostfixHeaderMap.cpp
>>>
>>> Modified:
>>> clang-tools-extra/trunk/include-fixer/find-all-symbols/HeaderMapCollector.cpp
>>> URL:
>>> http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/include-fixer/find-all-symbols/HeaderMapCollector.cpp?rev=318840&r1=318839&r2=318840&view=diff
>>>
>>> ==============================================================================
>>> ---
>>> clang-tools-extra/trunk/include-fixer/find-all-symbols/HeaderMapCollector.cpp
>>> (original)
>>> +++
>>> clang-tools-extra/trunk/include-fixer/find-all-symbols/HeaderMapCollector.cpp
>>> Wed Nov 22 07:38:23 2017
>>> @@ -13,6 +13,16 @@
>>>    namespace clang {
>>>    namespace find_all_symbols {
>>>    +HeaderMapCollector::HeaderMapCollector(
>>> +    const RegexHeaderMap *RegexHeaderMappingTable) {
>>> +  assert(RegexHeaderMappingTable);
>>> +  this->RegexHeaderMappingTable.reserve(RegexHeaderMappingTable->size());
>>> +  for (const auto &Entry : *RegexHeaderMappingTable) {
>>> +    this->RegexHeaderMappingTable.emplace_back(llvm::Regex(Entry.first),
>>> +                                               Entry.second);
>>> +  }
>>> +}
>>> +
>>>    llvm::StringRef
>>>    HeaderMapCollector::getMappedHeader(llvm::StringRef Header) const {
>>>      auto Iter = HeaderMappingTable.find(Header);
>>> @@ -20,11 +30,13 @@ HeaderMapCollector::getMappedHeader(llvm
>>>        return Iter->second;
>>>      // If there is no complete header name mapping for this header, check
>>> the
>>>      // regex header mapping.
>>> -  if (RegexHeaderMappingTable) {
>>> -    for (const auto &Entry : *RegexHeaderMappingTable) {
>>> -      if (llvm::Regex(Entry.first).match(Header))
>>> -        return Entry.second;
>>> -    }
>>> +  for (auto &Entry : RegexHeaderMappingTable) {
>>> +#ifndef NDEBUG
>>> +    std::string Dummy;
>>> +    assert(Entry.first.isValid(Dummy) && "Regex should never be
>>> invalid!");
>>> +#endif
>>> +    if (Entry.first.match(Header))
>>> +      return Entry.second;
>>>      }
>>>      return Header;
>>>    }
>>>
>>> Modified:
>>> clang-tools-extra/trunk/include-fixer/find-all-symbols/HeaderMapCollector.h
>>> URL:
>>> http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/include-fixer/find-all-symbols/HeaderMapCollector.h?rev=318840&r1=318839&r2=318840&view=diff
>>>
>>> ==============================================================================
>>> ---
>>> clang-tools-extra/trunk/include-fixer/find-all-symbols/HeaderMapCollector.h
>>> (original)
>>> +++
>>> clang-tools-extra/trunk/include-fixer/find-all-symbols/HeaderMapCollector.h
>>> Wed Nov 22 07:38:23 2017
>>> @@ -25,10 +25,8 @@ public:
>>>      typedef llvm::StringMap<std::string> HeaderMap;
>>>      typedef std::vector<std::pair<const char *, const char *>>
>>> RegexHeaderMap;
>>>    -  HeaderMapCollector() : RegexHeaderMappingTable(nullptr) {}
>>> -
>>> -  explicit HeaderMapCollector(const RegexHeaderMap
>>> *RegexHeaderMappingTable)
>>> -      : RegexHeaderMappingTable(RegexHeaderMappingTable) {}
>>> +  HeaderMapCollector() = default;
>>> +  explicit HeaderMapCollector(const RegexHeaderMap
>>> *RegexHeaderMappingTable);
>>>        void addHeaderMapping(llvm::StringRef OrignalHeaderPath,
>>>                            llvm::StringRef MappingHeaderPath) {
>>> @@ -47,8 +45,10 @@ private:
>>>      HeaderMap HeaderMappingTable;
>>>        // A map from header patterns to header names.
>>> -  // This is a reference to a hard-coded map.
>>> -  const RegexHeaderMap *const RegexHeaderMappingTable;
>>> +  // The header names are not owned. This is only threadsafe because the
>>> regexes
>>> +  // never fail.
>>> +  mutable std::vector<std::pair<llvm::Regex, const char *>>
>>> +      RegexHeaderMappingTable;
>>>    };
>>>      } // namespace find_all_symbols
>>>
>>> Modified:
>>> clang-tools-extra/trunk/include-fixer/find-all-symbols/STLPostfixHeaderMap.cpp
>>> URL:
>>> http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/include-fixer/find-all-symbols/STLPostfixHeaderMap.cpp?rev=318840&r1=318839&r2=318840&view=diff
>>>
>>> ==============================================================================
>>> ---
>>> clang-tools-extra/trunk/include-fixer/find-all-symbols/STLPostfixHeaderMap.cpp
>>> (original)
>>> +++
>>> clang-tools-extra/trunk/include-fixer/find-all-symbols/STLPostfixHeaderMap.cpp
>>> Wed Nov 22 07:38:23 2017
>>> @@ -314,10 +314,10 @@ const HeaderMapCollector::RegexHeaderMap
>>>          {"include/xlocale.h$", "<cstring>"},
>>>          {"bits/atomic_word.h$", "<memory>"},
>>>          {"bits/basic_file.h$", "<fstream>"},
>>> -      {"bits/c++allocator.h$", "<string>"},
>>> -      {"bits/c++config.h$", "<iosfwd>"},
>>> -      {"bits/c++io.h$", "<ios>"},
>>> -      {"bits/c++locale.h$", "<locale>"},
>>> +      {"bits/c\\+\\+allocator.h$", "<string>"},
>>> +      {"bits/c\\+\\+config.h$", "<iosfwd>"},
>>> +      {"bits/c\\+\\+io.h$", "<ios>"},
>>> +      {"bits/c\\+\\+locale.h$", "<locale>"},
>>>          {"bits/cpu_defines.h$", "<iosfwd>"},
>>>          {"bits/ctype_base.h$", "<locale>"},
>>>          {"bits/cxxabi_tweaks.h$", "<cxxabi.h>"},
>>>
>>>
>>> _______________________________________________
>>> cfe-commits mailing list
>>> cfe-commits at lists.llvm.org
>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>>>
>>>
>>> ---
>>> This email has been checked for viruses by AVG.
>>> http://www.avg.com
>>>
>>



More information about the cfe-commits mailing list