From 9168fe07f18f0946b7eeab305bc10c19109bb16f Mon Sep 17 00:00:00 2001 From: amzn-mike <80125227+amzn-mike@users.noreply.github.com> Date: Thu, 3 Feb 2022 08:50:35 -0600 Subject: [PATCH] Asset Processor: Zero analysis dependency fingerprint verification (#7292) * Move modtime scanning tests out of APM tests file and into its own file. Changes were kept to a minimum to get things compiling, this is just a move of code Signed-off-by: amzn-mike <80125227+amzn-mike@users.noreply.github.com> * Fix rebase compile errors Signed-off-by: amzn-mike <80125227+amzn-mike@users.noreply.github.com> * Verify dependency fingerprints during Zero Analysis. This fixes an issue where dependencies that weren't finished processing when AP shuts down would not resume when AP is started back up due to Zero Analysis ignoring dependencies when determining files to skip. Added unit test for verification Signed-off-by: amzn-mike <80125227+amzn-mike@users.noreply.github.com> * Fix compile error, make 17 a constexpr Signed-off-by: amzn-mike <80125227+amzn-mike@users.noreply.github.com> * Fix compile error Signed-off-by: amzn-mike <80125227+amzn-mike@users.noreply.github.com> --- .../AssetManager/assetProcessorManager.cpp | 61 ++++++++++------ .../AssetManager/assetProcessorManager.h | 1 + .../assetmanager/ModtimeScanningTests.cpp | 73 +++++++++++++++++++ 3 files changed, 112 insertions(+), 23 deletions(-) diff --git a/Code/Tools/AssetProcessor/native/AssetManager/assetProcessorManager.cpp b/Code/Tools/AssetProcessor/native/AssetManager/assetProcessorManager.cpp index beb82dfcab..d1b078c59d 100644 --- a/Code/Tools/AssetProcessor/native/AssetManager/assetProcessorManager.cpp +++ b/Code/Tools/AssetProcessor/native/AssetManager/assetProcessorManager.cpp @@ -2942,11 +2942,20 @@ namespace AssetProcessor // File is a source file that has been processed before AZStd::string fingerprintFromDatabase = sourceFileItr->m_analysisFingerprint.toUtf8().data(); AZStd::string_view builderEntries(fingerprintFromDatabase.begin() + s_lengthOfUuid + 1, fingerprintFromDatabase.end()); + AZStd::string_view dependencyFingerprint(fingerprintFromDatabase.begin(), fingerprintFromDatabase.begin() + s_lengthOfUuid); int numBuildersEmittingSourceDependencies = 0; if (!fingerprintFromDatabase.empty() && AreBuildersUnchanged(builderEntries, numBuildersEmittingSourceDependencies)) { // Builder(s) have not changed since last time + AZStd::string currentFingerprint = ComputeRecursiveDependenciesFingerprint( + fileInfo.m_filePath.toUtf8().constData(), sourceFileItr->m_sourceDatabaseName.toUtf8().constData()); + + if(dependencyFingerprint != currentFingerprint) + { + // Dependencies have changed + return false; + } // Success - we can skip this file, nothing has changed! // Remove it from the list of to-be-processed files, otherwise the AP will assume the file was deleted @@ -4311,6 +4320,32 @@ namespace AssetProcessor } } + AZStd::string AssetProcessorManager::ComputeRecursiveDependenciesFingerprint(const AZStd::string& fileAbsolutePath, const AZStd::string& fileDatabaseName) + { + AZStd::string concatenatedFingerprints; + + // QSet is not ordered. + SourceFilesForFingerprintingContainer knownDependenciesAbsolutePaths; + // this automatically adds the input file to the list: + QueryAbsolutePathDependenciesRecursive(QString::fromUtf8(fileDatabaseName.c_str()), knownDependenciesAbsolutePaths, + AzToolsFramework::AssetDatabase::SourceFileDependencyEntry::DEP_Any, false); + AddMetadataFilesForFingerprinting(QString::fromUtf8(fileAbsolutePath.c_str()), knownDependenciesAbsolutePaths); + + // reserve 17 chars for each since its a 64 bit hex number, and then one more for the dash inbetween each. + constexpr int bytesPerFingerprint = (sizeof(AZ::u64) * 2) + 1; // 2 HEX characters per byte +1 for the `-` we will add between each fingerprint + concatenatedFingerprints.reserve((knownDependenciesAbsolutePaths.size() * bytesPerFingerprint)); + + for (const auto& element : knownDependenciesAbsolutePaths) + { + // if its a placeholder then don't bother hitting the disk to find it. + concatenatedFingerprints.append(AssetUtilities::GetFileFingerprint(element.first, element.second)); + concatenatedFingerprints.append("-"); + } + + // to keep this from growing out of hand, we don't use the full string, we use a hash of it: + return AZ::Uuid::CreateName(concatenatedFingerprints.c_str()).ToString(); + } + void AssetProcessorManager::FinishAnalysis(AZStd::string fileToCheck) { using namespace AzToolsFramework::AssetDatabase; @@ -4378,29 +4413,9 @@ namespace AssetProcessor if (found) { // construct the analysis fingerprint - // the format for this data is "modtimefingerprint:builder0:builder1:builder2:...:buildern" - source.m_analysisFingerprint.clear(); - // compute mod times: - // get the appropriate modtimes: - AZStd::string modTimeArray; + // the format for this data is "hashfingerprint:builder0:builder1:builder2:...:buildern" + source.m_analysisFingerprint = ComputeRecursiveDependenciesFingerprint(fileToCheck, analysisTracker->m_databaseSourceName); - // QSet is not ordered. - SourceFilesForFingerprintingContainer knownDependenciesAbsolutePaths; - // this automatically adds the input file to the list: - QueryAbsolutePathDependenciesRecursive(QString::fromUtf8(analysisTracker->m_databaseSourceName.c_str()), knownDependenciesAbsolutePaths, SourceFileDependencyEntry::DEP_Any, false); - AddMetadataFilesForFingerprinting(QString::fromUtf8(fileToCheck.c_str()), knownDependenciesAbsolutePaths); - - // reserve 17 chars for each since its a 64 bit hex number, and then one more for the dash inbetween each. - modTimeArray.reserve((knownDependenciesAbsolutePaths.size() * 17)); - - for (const auto& element : knownDependenciesAbsolutePaths) - { - // if its a placeholder then don't bother hitting the disk to find it. - modTimeArray.append(AssetUtilities::GetFileFingerprint(element.first, element.second)); - modTimeArray.append("-"); - } - // to keep this from growing out of hand, we don't use the full string, we use a hash of it: - source.m_analysisFingerprint = AZ::Uuid::CreateName(modTimeArray.c_str()).ToString(); for (const AZ::Uuid& builderID : analysisTracker->m_buildersInvolved) { source.m_analysisFingerprint.append(":"); @@ -4423,7 +4438,7 @@ namespace AssetProcessor const ScanFolderInfo* scanFolder = m_platformConfig->GetScanFolderForFile(fileToCheck.c_str()); scanFolderPk = aznumeric_cast(scanFolder->ScanFolderID()); - m_platformConfig->ConvertToRelativePath(fileToCheck.c_str(), scanFolder, databaseSourceName); + PlatformConfiguration::ConvertToRelativePath(fileToCheck.c_str(), scanFolder, databaseSourceName); } // Record the modtime for the file so we know we processed it diff --git a/Code/Tools/AssetProcessor/native/AssetManager/assetProcessorManager.h b/Code/Tools/AssetProcessor/native/AssetManager/assetProcessorManager.h index fe77396760..eb40b476b1 100644 --- a/Code/Tools/AssetProcessor/native/AssetManager/assetProcessorManager.h +++ b/Code/Tools/AssetProcessor/native/AssetManager/assetProcessorManager.h @@ -485,6 +485,7 @@ namespace AssetProcessor }; void ComputeBuilderDirty(); + AZStd::string ComputeRecursiveDependenciesFingerprint(const AZStd::string& fileAbsolutePath, const AZStd::string& fileDatabaseName); AZStd::unordered_map m_builderDataCache; bool m_buildersAddedOrRemoved = true; //< true if any new builders exist. If this happens we actually need to re-analyze everything. bool m_anyBuilderChange = true; diff --git a/Code/Tools/AssetProcessor/native/tests/assetmanager/ModtimeScanningTests.cpp b/Code/Tools/AssetProcessor/native/tests/assetmanager/ModtimeScanningTests.cpp index 6b3124ca00..5552871d1f 100644 --- a/Code/Tools/AssetProcessor/native/tests/assetmanager/ModtimeScanningTests.cpp +++ b/Code/Tools/AssetProcessor/native/tests/assetmanager/ModtimeScanningTests.cpp @@ -516,6 +516,79 @@ namespace UnitTests ASSERT_EQ(m_data->m_processResults.size(), 2); } + TEST_F(ModtimeScanningTest, AssetProcessorIsRestartedBeforeDependencyIsProcessed_DependencyIsProcessedOnStart) + { + using namespace AzToolsFramework::AssetSystem; + auto theFile = m_data->m_absolutePath[1].toUtf8(); + const char* theFileString = theFile.constData(); + + SetFileContents(theFileString, "hello world"); + + // Enable the features we're testing + m_assetProcessorManager->SetEnableModtimeSkippingFeature(true); + AssetUtilities::SetUseFileHashOverride(true, true); + + QSet filePaths = BuildFileSet(); + SimulateAssetScanner(filePaths); + + // Even though we're only updating one file, we're expecting 2 createJob calls because our test file is a dependency that triggers + // the other test file to process as well + ExpectWork(2, 2); + + // Sort the results and process the first one, which should always be the modtimeTestDependency.txt file + // which is the same file we modified above. modtimeTestFile.txt depends on this file but we're not going to process it yet. + { + std::sort( + m_data->m_processResults.begin(), m_data->m_processResults.end(), + [](decltype(m_data->m_processResults[0])& left, decltype(left)& right) + { + return left.m_jobEntry.m_databaseSourceName < right.m_jobEntry.m_databaseSourceName; + }); + + const auto& processResult = m_data->m_processResults[0]; + auto file = + QDir(processResult.m_destinationPath).absoluteFilePath(processResult.m_jobEntry.m_databaseSourceName.toLower() + ".arc1"); + m_data->m_productPaths.emplace( + QDir(processResult.m_jobEntry.m_watchFolderPath) + .absoluteFilePath(processResult.m_jobEntry.m_databaseSourceName) + .toUtf8() + .constData(), + file); + + // Create the file on disk + ASSERT_TRUE(UnitTestUtils::CreateDummyFile(file, "products.")); + + AssetBuilderSDK::ProcessJobResponse response; + response.m_resultCode = AssetBuilderSDK::ProcessJobResult_Success; + response.m_outputProducts.push_back(AssetBuilderSDK::JobProduct(file.toUtf8().constData(), AZ::Uuid::CreateNull(), 1)); + + using JobEntry = AssetProcessor::JobEntry; + + QMetaObject::invokeMethod( + m_assetProcessorManager.get(), "AssetProcessed", Qt::QueuedConnection, Q_ARG(JobEntry, processResult.m_jobEntry), + Q_ARG(AssetBuilderSDK::ProcessJobResponse, response)); + } + + ASSERT_TRUE(BlockUntilIdle(5000)); + + // Shutdown and restart the APM + m_assetProcessorManager.reset(); + m_assetProcessorManager = AZStd::make_unique(m_config.get()); + + SetUpAssetProcessorManager(); + + m_data->m_mockBuilderInfoHandler.m_createJobsCount = 0; + m_data->m_processResults.clear(); + m_data->m_deletedSources.clear(); + + // Re-run the scanner on our files + filePaths = BuildFileSet(); + SimulateAssetScanner(filePaths); + + // Expect processing to resume on the job we didn't process before + ExpectWork(1, 1); + } + void DeleteTest::SetUp() { AssetProcessorManagerTest::SetUp();