[clang-tools-extra] r318840 - [FindAllSymbols] Cache regexes, creating them is expensive
Benjamin Kramer via cfe-commits
cfe-commits at lists.llvm.org
Thu Nov 23 09:47:37 PST 2017
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