Skip to content

Commit 637d5be

Browse files
authored
Fix #14995 (Suppression file name cache) (#8684)
Implement a file name cache per suppression to avoid calling PathMatch::match again and again for the same suppression and file name combination. Local benchmark running cppcheck with misra c-2012 has been performed. **Before** 10min 34sec **After** 1min 36sec In this test it was a few header files producing the majority of the error messages, which is why the cache speeds it up by a lot.
1 parent 8357500 commit 637d5be

3 files changed

Lines changed: 62 additions & 1 deletion

File tree

lib/suppressions.cpp

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -423,7 +423,7 @@ SuppressionList::Suppression::Result SuppressionList::Suppression::isSuppressed(
423423
if (!thisAndNextLine || lineNumber + 1 != errmsg.lineNumber)
424424
return Result::None;
425425
}
426-
if (!fileName.empty() && !PathMatch::match(fileName, errmsg.getFileName()))
426+
if (!fileName.empty() && !isFileNameMatch(errmsg.getFileName()))
427427
return Result::None;
428428
if (hash > 0 && hash != errmsg.hash)
429429
return Result::Checked;
@@ -456,6 +456,21 @@ SuppressionList::Suppression::Result SuppressionList::Suppression::isSuppressed(
456456
return Result::Matched;
457457
}
458458

459+
bool SuppressionList::Suppression::isFileNameMatch(const std::string &errorFileName) const
460+
{
461+
const auto it = mFileNameMatchCache.find(errorFileName);
462+
if (it != mFileNameMatchCache.end())
463+
return it->second;
464+
465+
const bool result = PathMatch::match(fileName, errorFileName);
466+
467+
if (mFileNameMatchCache.size() >= mFileNameMatchCacheMaxEntries)
468+
mFileNameMatchCache.clear();
469+
470+
mFileNameMatchCache.emplace(errorFileName, result);
471+
return result;
472+
}
473+
459474
bool SuppressionList::Suppression::isMatch(const SuppressionList::ErrorMessage &errmsg)
460475
{
461476
switch (isSuppressed(errmsg)) {

lib/suppressions.h

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -168,6 +168,12 @@ class CPPCHECKLIB SuppressionList {
168168
bool isPolyspace{};
169169

170170
enum : std::int8_t { NO_LINE = -1 };
171+
172+
private:
173+
bool isFileNameMatch(const std::string &errorFileName) const;
174+
175+
static constexpr std::size_t mFileNameMatchCacheMaxEntries = 256;
176+
mutable std::map<std::string, bool> mFileNameMatchCache;
171177
};
172178

173179
/**

test/cli/performance_test.py

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -438,3 +438,43 @@ def test_slow_many_headers(tmpdir):
438438
cppcheck(['-DCONFIG0', c_file])
439439
end = time.perf_counter_ns()
440440
assert end - start < 2 * 10**9 # max 2 sec
441+
442+
443+
@pytest.mark.skipif(sys.platform == 'darwin', reason='GitHub macOS runners are too slow')
444+
@pytest.mark.timeout(10)
445+
def test_large_number_of_violations_and_suppressions(tmpdir):
446+
filename_main = os.path.join(tmpdir, 'main.c')
447+
# This name causes the PathMatch::match() to iterate ~70 times, which is not unrealistic for a header file placed in subdirs.
448+
# The number of iterations also depends on how the suppressions are written.
449+
header_name = 'long_filename_to_simulate_a_more_realistic_header_placed_in_subdirs.h'
450+
header_file = os.path.join(tmpdir, header_name)
451+
suppressions_file = os.path.join(tmpdir, 'suppressions.txt')
452+
453+
# Create a main file that includes a header and returns 0.
454+
with open(filename_main, "w") as f:
455+
f.write(f'#include "{header_name}"\n\n')
456+
f.write('int main() \n')
457+
f.write('{\n')
458+
f.write(' return 0;\n')
459+
f.write('}\n')
460+
461+
# Create a header file with macro definitions that violates misra because they start with an underscore.
462+
with open(header_file, "w") as f:
463+
f.write(f'#ifndef {header_name.upper().replace(".", "_")}\n')
464+
f.write(f'#define {header_name.upper().replace(".", "_")}\n')
465+
f.write('\n')
466+
for i in range(5000):
467+
f.write(f'#define _FOO{i} {i}\n')
468+
f.write('\n')
469+
f.write(f'#endif // {header_name.upper().replace(".", "_")}\n')
470+
471+
# Create a suppressions file that suppresses the misra violation for the macros in the header file
472+
with open(suppressions_file, "w") as f:
473+
f.write(f'misra-c2012-2.5:{header_name}\n')
474+
f.write(f'misra-c2012-21.1:{header_name}\n')
475+
# Create other suppressions that don't match the file name to test that iterating the suppressions are faster with cache
476+
for i in range(300):
477+
f.write(f'misra-c2012-2.5:**/{i}/{header_name}\n')
478+
f.write(f'misra-c2012-21.1:**/{i}/{header_name}\n')
479+
480+
cppcheck([filename_main] + ['--addon=misra.py', '--check-level=exhaustive', '--enable=all', f'--suppressions-list={suppressions_file}'])

0 commit comments

Comments
 (0)