[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 10:50:44 PST 2017
I'm afraid I can't really help you here. You can try twiddling the
code a bit by creating the pair explicitly and moving it into the
vector, just to avoid the brokenness in the standard library. Not sure
if that will help though.
On Thu, Nov 23, 2017 at 7:05 PM, Krzysztof Parzyszek
<kparzysz at codeaurora.org> wrote:
> 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