Author: vsapsai Date: Wed Aug 28 16:31:32 2019 New Revision: 370274 URL: http://llvm.org/viewvc/llvm-project?rev=370274&view=rev Log: [Modules] Fix rebuilding an updated module for each of its consumers.
Marking a module for a rebuild when its signature differs from the expected one causes redundant module rebuilds for incremental builds. When a module is updated, its signature changes. But its consumers still have the old signature and loading them will result in signature mismatches. It will correctly cause the rebuilds for the consumers but we don't need to rebuild the common module for each of them as it is already up to date. In practice this bug causes longer build times. We are doing more work than required and only a single process can build a module, so parallel builds degrade to a single-process mode where extra processes are just waiting on a file lock. Fix by not marking a module dependency for a rebuild on signature mismatch. We'll check if it is up to date when we load it. rdar://problem/50212358 Reviewers: dexonsmith, bruno, rsmith Reviewed By: dexonsmith, bruno Subscribers: jkorous, ributzka, cfe-commits, aprantl Differential Revision: https://reviews.llvm.org/D66907 Added: cfe/trunk/test/Modules/Inputs/implicit-invalidate-common/ cfe/trunk/test/Modules/Inputs/implicit-invalidate-common/A.h cfe/trunk/test/Modules/Inputs/implicit-invalidate-common/B.h cfe/trunk/test/Modules/Inputs/implicit-invalidate-common/Common.h cfe/trunk/test/Modules/Inputs/implicit-invalidate-common/module.modulemap cfe/trunk/test/Modules/implicit-invalidate-common.c Modified: cfe/trunk/lib/Serialization/ModuleManager.cpp Modified: cfe/trunk/lib/Serialization/ModuleManager.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ModuleManager.cpp?rev=370274&r1=370273&r2=370274&view=diff ============================================================================== --- cfe/trunk/lib/Serialization/ModuleManager.cpp (original) +++ cfe/trunk/lib/Serialization/ModuleManager.cpp Wed Aug 28 16:31:32 2019 @@ -204,13 +204,8 @@ ModuleManager::addModule(StringRef FileN // Read the signature eagerly now so that we can check it. Avoid calling // ReadSignature unless there's something to check though. if (ExpectedSignature && checkSignature(ReadSignature(NewModule->Data), - ExpectedSignature, ErrorStr)) { - // Try to remove the buffer. If it can't be removed, then it was already - // validated by this process. - if (!getModuleCache().tryToDropPCM(NewModule->FileName)) - FileMgr.invalidateCache(NewModule->File); + ExpectedSignature, ErrorStr)) return OutOfDate; - } // We're keeping this module. Store it everywhere. Module = Modules[Entry] = NewModule.get(); Added: cfe/trunk/test/Modules/Inputs/implicit-invalidate-common/A.h URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/implicit-invalidate-common/A.h?rev=370274&view=auto ============================================================================== --- cfe/trunk/test/Modules/Inputs/implicit-invalidate-common/A.h (added) +++ cfe/trunk/test/Modules/Inputs/implicit-invalidate-common/A.h Wed Aug 28 16:31:32 2019 @@ -0,0 +1,2 @@ +// A +#include "Common.h" Added: cfe/trunk/test/Modules/Inputs/implicit-invalidate-common/B.h URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/implicit-invalidate-common/B.h?rev=370274&view=auto ============================================================================== --- cfe/trunk/test/Modules/Inputs/implicit-invalidate-common/B.h (added) +++ cfe/trunk/test/Modules/Inputs/implicit-invalidate-common/B.h Wed Aug 28 16:31:32 2019 @@ -0,0 +1,2 @@ +// B +#include "Common.h" Added: cfe/trunk/test/Modules/Inputs/implicit-invalidate-common/Common.h URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/implicit-invalidate-common/Common.h?rev=370274&view=auto ============================================================================== --- cfe/trunk/test/Modules/Inputs/implicit-invalidate-common/Common.h (added) +++ cfe/trunk/test/Modules/Inputs/implicit-invalidate-common/Common.h Wed Aug 28 16:31:32 2019 @@ -0,0 +1 @@ +// Common Added: cfe/trunk/test/Modules/Inputs/implicit-invalidate-common/module.modulemap URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/implicit-invalidate-common/module.modulemap?rev=370274&view=auto ============================================================================== --- cfe/trunk/test/Modules/Inputs/implicit-invalidate-common/module.modulemap (added) +++ cfe/trunk/test/Modules/Inputs/implicit-invalidate-common/module.modulemap Wed Aug 28 16:31:32 2019 @@ -0,0 +1,3 @@ +module A { header "A.h" } +module B { header "B.h" } +module Common { header "Common.h" } Added: cfe/trunk/test/Modules/implicit-invalidate-common.c URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/implicit-invalidate-common.c?rev=370274&view=auto ============================================================================== --- cfe/trunk/test/Modules/implicit-invalidate-common.c (added) +++ cfe/trunk/test/Modules/implicit-invalidate-common.c Wed Aug 28 16:31:32 2019 @@ -0,0 +1,36 @@ +// REQUIRES: shell +// RUN: rm -rf %t +// RUN: mkdir -p %t/implicit-invalidate-common +// RUN: cp -r %S/Inputs/implicit-invalidate-common %t/ +// RUN: echo '#include "A.h"' > %t/A.c +// RUN: echo '#include "B.h"' > %t/B.c + +// Build with an empty module cache. Module 'Common' should be built only once. +// +// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t/Cache \ +// RUN: -fsyntax-only -I %t/implicit-invalidate-common -Rmodule-build \ +// RUN: %t/A.c 2> %t/initial_build.txt +// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t/Cache \ +// RUN: -fsyntax-only -I %t/implicit-invalidate-common -Rmodule-build \ +// RUN: %t/B.c 2>> %t/initial_build.txt +// RUN: FileCheck %s --implicit-check-not "remark:" --input-file %t/initial_build.txt + +// Update module 'Common' and build with the populated module cache. Module +// 'Common' still should be built only once. Note that we are using the same +// flags for A.c and B.c to avoid building Common.pcm at different paths. +// +// RUN: echo ' // ' >> %t/implicit-invalidate-common/Common.h +// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t/Cache \ +// RUN: -fsyntax-only -I %t/implicit-invalidate-common -Rmodule-build \ +// RUN: %t/A.c 2> %t/incremental_build.txt +// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t/Cache \ +// RUN: -fsyntax-only -I %t/implicit-invalidate-common -Rmodule-build \ +// RUN: %t/B.c 2>> %t/incremental_build.txt +// RUN: FileCheck %s --implicit-check-not "remark:" --input-file %t/incremental_build.txt + +// CHECK: remark: building module 'A' +// CHECK: remark: building module 'Common' +// CHECK: remark: finished building module 'Common' +// CHECK: remark: finished building module 'A' +// CHECK: remark: building module 'B' +// CHECK: remark: finished building module 'B' _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits