aaron.ballman added inline comments.
================
Comment at: clang/lib/Basic/Attributes.cpp:101-103
+ for (ParsedAttrInfoRegistry::iterator it = ParsedAttrInfoRegistry::begin(),
+ ie = ParsedAttrInfoRegistry::end();
+ it != ie; ++it) {
----------------
john.brawn wrote:
> aaron.ballman wrote:
> > Range-based for loop? Also, `it` and `ie` don't meet the usual naming
> > conventions.
> >
> > One thing I'm not keen on with this is the performance hit. We spent a
> > decent amount of effort making this lookup fast and it's now a linear
> > search through all of the attributes and requires memory allocations and
> > deallocations to perform the search.
> Yes, I've done some experiments and in the pathological case (lots of trivial
> functions with the xray_log_args attribute) the slowdown is noticeable. I've
> done some tinkering and I think the best way to resolve this is to first use
> a generated function (i.e. something like the current getAttrKind) to look up
> the attributes that are compiled into clang, then if that fails look in the
> registry to find a match.
I think that approach makes sense and is something we should probably do up
front. It doesn't seem like `llvm::Registry` has a way for us to mark where the
plugin attributes start to allow for quickly searching for plugin attributes in
the fallback case.
I don't see a particularly good way around the memory allocations, however.
Ideally, this could be something that's arena allocated on the `ASTContext`
rather than using `unique_ptr`, but I think that would be tricky here because
of library layering. After switching the lookup functionality, I'd be curious
to know if there is a noticeable performance degredation still, or if that
falls out into the noise.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D31338/new/
https://reviews.llvm.org/D31338
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
