Page MenuHomePhabricator

[Coverage][NFC] Remove skipped region after added into MappingRegions
AbandonedPublic

Authored by zequanwu on Sep 14 2020, 4:01 PM.

Details

Reviewers
vsk
Summary

There is no need to scan through all SkippedRegions when some of them are already added into MappingRegions.

Diff Detail

Event Timeline

zequanwu created this revision.Sep 14 2020, 4:01 PM
Herald added a project: Restricted Project. · View Herald TranscriptSep 14 2020, 4:01 PM
Herald added a subscriber: cfe-commits. · View Herald Transcript
zequanwu requested review of this revision.Sep 14 2020, 4:01 PM
vsk added a comment.Sep 18 2020, 2:18 PM

Unfortunately it does look like the work done in gatherSkippedRegions is O(n^2) in the number of functions, at the moment. If the goal is to speed it up, it'd be good to grab some performance numbers for some representative compile unit (the sqlite3 amalgamation is my go-to for this sort of thing).

clang/lib/CodeGen/CoverageMappingGen.cpp
353

In the worst case, the work done in gatherSkippedRegions may still be n + (n-1) + ... ~ O(n^2) (I'm subtracting 1 each time the skipped regions for a function are erased).

356

It'd probably be good to split the lambda out into a separate/named helper.

vsk added a comment.Sep 18 2020, 2:24 PM

(Depending on what the potential performance gains look like, it might be advisable to keep things simple with the current O(n^2) approach. Optimizing things can carry some risk -- not sure if we've already ruled this out, but e.g. perhaps there's some edge case with nested functions where we maybe don't want to erase skipped regions eagerly.)

zequanwu abandoned this revision.Sep 28 2020, 10:41 AM
zequanwu added inline comments.
clang/lib/CodeGen/CoverageMappingGen.cpp
353

Yeah, just notice that.