This is an archive of the discontinued LLVM Phabricator instance.

[Sema][NFCI] Don't allocate storage for the various CorrectionCandidateCallback unless we are going to do some typo correction
ClosedPublic

Authored by riccibruno on Mar 1 2019, 6:33 AM.

Details

Summary

The various CorrectionCandidateCallbacks are currently heap-allocated unconditionally. This was needed because of delayed typo correction. However these allocations represent currently 15.4% of all allocations (number of allocations) when parsing all of Boost (!), mostly because of ParseCastExpression, ParseStatementOrDeclarationAfterAttrtibutes and isCXXDeclarationSpecifier. Note that all of these callback objects are small. Let's not do this.

Instead initially allocate the callback on the stack, and only do a heap allocation if we are going to do some typo correction. Do this by:

  1. Adding a clone function to each callback, which will do a polymorphic clone of the callback. This clone function is required to be implemented by every callback (of which there is a fair amount). Make sure this is the case by making it pure virtual.
  2. Use this clone function when we are going to try to correct a typo.

This additionally cut the time of -fsyntax-only on all of Boost by 0.5% (not that much, but still something). No functional changes intended.

Diff Detail

Repository
rC Clang

Event Timeline

riccibruno created this revision.Mar 1 2019, 6:33 AM
riccibruno retitled this revision from [Sema][NFCI] Don't heap-allocate the various CorrectionCandidateCallback unless we are going to do some typo correction to [Sema][NFCI] Don't allocate storage for the various CorrectionCandidateCallback unless we are going to do some typo correction.Mar 18 2019, 5:53 AM
rnk accepted this revision.Mar 18 2019, 1:57 PM

I think this is worth the complexity of the repeated clone methods. lgtm

lib/Sema/SemaType.cpp
5911

These changes are unrelated, but good.

This revision is now accepted and ready to land.Mar 18 2019, 1:57 PM
This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptMar 25 2019, 10:07 AM