From 57d780929741e3d09f60b252a26ad12fb645254e Mon Sep 17 00:00:00 2001 From: David Steele Date: Sat, 3 Nov 2018 19:52:46 -0400 Subject: [PATCH] Improve efficiency of code generation. Code generation saved files even when they had not changed, which often caused code generation cascades. So, don't save files unless they have changed. Use rsync to determine which files have changed since the last test run. The manifest of changed files is saved and not removed until all code generation and builds have completed. If an error occurs the work will be redone on the next run. The eventual goal is to do all the builds from the test/repo directory created by rsync but for now it is only used to track changes. --- .gitignore | 1 + build/lib/pgBackRestBuild/Build.pm | 25 +++- doc/xml/release.xml | 2 +- libc/build/lib/pgBackRestLibC/Build.pm | 51 +++++++- test/lib/pgBackRestTest/Common/BuildTest.pm | 3 + test/lib/pgBackRestTest/Common/CiTest.pm | 3 +- test/test.pl | 137 ++++++++++++++++---- 7 files changed, 186 insertions(+), 36 deletions(-) diff --git a/.gitignore b/.gitignore index f106e7f6e..4dab0bf7b 100644 --- a/.gitignore +++ b/.gitignore @@ -1,3 +1,4 @@ **/*~ *~ *.swp +.DS_Store diff --git a/build/lib/pgBackRestBuild/Build.pm b/build/lib/pgBackRestBuild/Build.pm index 776923bbf..9aed688d2 100644 --- a/build/lib/pgBackRestBuild/Build.pm +++ b/build/lib/pgBackRestBuild/Build.pm @@ -10,6 +10,7 @@ use English '-no_match_vars'; use Exporter qw(import); our @EXPORT = qw(); +use File::Basename qw(basename); use Storable qw(dclone); use pgBackRest::Common::Log; @@ -37,6 +38,9 @@ sub buildAll my $oStorage = new pgBackRest::Storage::Local( $strBuildPath, new pgBackRest::Storage::Posix::Driver({bFileSync => false, bPathSync => false})); + # List of files actually built + my @stryBuilt; + # Build and output source code #------------------------------------------------------------------------------------------------------------------------------- foreach my $strBuild (sort(keys(%{$rhBuild}))) @@ -162,14 +166,27 @@ sub buildAll $strExt = $strFileType eq BLD_C ? $strFileExt : "${strFileExt}h"; } - # Save the file - $oStorage->put("${strPath}/${strFile}.auto.${strExt}", trim($rhSource->{$strFileType}) . "\n"); + # Save the file if it has not changed + my $strBuilt = "${strPath}/${strFile}.auto.${strExt}"; + my $bSave = true; + my $oFile = $oStorage->openRead($strBuilt, {bIgnoreMissing => true}); + + if (defined($oFile) && ${$oStorage->get($oFile)} eq (trim($rhSource->{$strFileType}) . "\n")) + { + $bSave = false; + } + + if ($bSave) + { + $oStorage->put($strBuilt, trim($rhSource->{$strFileType}) . "\n"); + push(@stryBuilt, basename($strBuildPath) . "/${strBuilt}"); + } } } } - # Return build hash to caller for further processing - return $rhBuild; + # Return list of files built + return @stryBuilt; } push @EXPORT, qw(buildAll); diff --git a/doc/xml/release.xml b/doc/xml/release.xml index 988a04179..b27fce005 100644 --- a/doc/xml/release.xml +++ b/doc/xml/release.xml @@ -91,7 +91,7 @@ -

Test speed improvements. Mount tmpfs in Vagrantfile instead test.pl. Preserve contents of C unit test build directory between test.pl executions.

+

Test speed improvements. Mount tmpfs in Vagrantfile instead test.pl. Preserve contents of C unit test build directory between test.pl executions. Improve efficiency of code generation.

 diff --git a/libc/build/lib/pgBackRestLibC/Build.pm b/libc/build/lib/pgBackRestLibC/Build.pm index 48f76928f..b3490e857 100644 --- a/libc/build/lib/pgBackRestLibC/Build.pm +++ b/libc/build/lib/pgBackRestLibC/Build.pm @@ -19,13 +19,13 @@ use lib dirname($0) . '/../lib'; use pgBackRest::Common::Log; use pgBackRest::Common::String; -use pgBackRestBuild::Config::Data; use pgBackRest::Storage::Local; use pgBackRest::Storage::Posix::Driver; use pgBackRest::Version; use pgBackRestBuild::Build; use pgBackRestBuild::Build::Common; +use pgBackRestBuild::Config::Data; use pgBackRestBuild::Error::Data; #################################################################################################################################### @@ -36,6 +36,35 @@ use constant BLD_EXPORTTYPE_CONSTANT => 'constant use constant LIB_AUTO_NAME => 'LibCAuto'; +#################################################################################################################################### +# Save contents to a file if the file is missing or the contents are different. This saves write IO and prevents the timestamp from +# changing. +#################################################################################################################################### +sub buildPutDiffers +{ + my $oStorage = shift; + my $strFile = shift; + my $strContents = shift; + + # Attempt to load the file + my $bSave = true; + my $oFile = $oStorage->openRead($strFile, {bIgnoreMissing => true}); + + # If file was found see if the content is the same + if (defined($oFile) && ${$oStorage->get($oFile)} eq $strContents) + { + $bSave = false; + } + + # Save if the contents are different or missing + if ($bSave) + { + $oStorage->put($strFile, $strContents); + } + + return $bSave; +} + #################################################################################################################################### # Static exports #################################################################################################################################### @@ -139,6 +168,9 @@ sub buildXsAll { my $strBuildPath = shift; + # List of files built + my @stryBuilt; + # Storage my $oStorage = new pgBackRest::Storage::Local( $strBuildPath, new pgBackRest::Storage::Posix::Driver({bFileSync => false, bPathSync => false})); @@ -311,7 +343,12 @@ sub buildXsAll "1;\n"; # Save the file - $oStorage->put('../lib/' . BACKREST_NAME . '/' . LIB_AUTO_NAME . '.pm', $strContent); + my $strFile = 'lib/' . BACKREST_NAME . '/' . LIB_AUTO_NAME . '.pm'; + + if (buildPutDiffers($oStorage, "../${strFile}", $strContent)) + { + push(@stryBuilt, $strFile); + } # Build error file #------------------------------------------------------------------------------------------------------------------------------- @@ -364,7 +401,15 @@ sub buildXsAll "\n" . "1;\n"; - $oStorage->put('../lib/' . BACKREST_NAME . '/Common/ExceptionAuto.pm', $strContent); + $strFile = 'lib/' . BACKREST_NAME . '/Common/ExceptionAuto.pm'; + + if (buildPutDiffers($oStorage, "../${strFile}", $strContent)) + { + push(@stryBuilt, $strFile); + } + + # Return list of files built + return @stryBuilt; } push @EXPORT, qw(buildXsAll); diff --git a/test/lib/pgBackRestTest/Common/BuildTest.pm b/test/lib/pgBackRestTest/Common/BuildTest.pm index 19f9b20c6..af459cdf8 100644 --- a/test/lib/pgBackRestTest/Common/BuildTest.pm +++ b/test/lib/pgBackRestTest/Common/BuildTest.pm @@ -42,6 +42,9 @@ sub buildPutDiffers { $oStorage->put($strFile, $strContents); } + + # Was the file saved? + return $bSave; } push @EXPORT, qw(buildPutDiffers); diff --git a/test/lib/pgBackRestTest/Common/CiTest.pm b/test/lib/pgBackRestTest/Common/CiTest.pm index 684d58feb..530345f44 100644 --- a/test/lib/pgBackRestTest/Common/CiTest.pm +++ b/test/lib/pgBackRestTest/Common/CiTest.pm @@ -24,6 +24,7 @@ use pgBackRest::Common::Log; use pgBackRest::Common::String; use pgBackRest::Version; +use pgBackRestTest::Common::BuildTest; use pgBackRestTest::Common::ContainerTest; use pgBackRestTest::Common::DefineTest; use pgBackRestTest::Common::ExecuteTest; @@ -129,7 +130,7 @@ sub process "script:\n" . " - " . BACKREST_EXE . "/test/travis.pl \${PGB_CI?}\n"; - $self->{oStorage}->put('.travis.yml', $strConfig); + buildPutDiffers($self->{oStorage}, '.travis.yml', $strConfig); # Return from function and log return values if any return logDebugReturn($strOperation); diff --git a/test/test.pl b/test/test.pl index 56cd285fa..09b6555f5 100755 --- a/test/test.pl +++ b/test/test.pl @@ -358,7 +358,51 @@ eval ################################################################################################################################ if (!defined($iVmId)) { + # Make a copy of the repo to track which files have been changed. Eventually all builds will be done from this directory. + #--------------------------------------------------------------------------------------------------------------------------- + my $strRepoCachePath = "${strTestPath}/repo"; + my $strRepoCacheManifest = 'repo.manifest'; + + # Create the repo path -- this should hopefully prevent obvious rsync errors below + $oStorageTest->pathCreate("${strTestPath}/repo", {strMode => '0770', bIgnoreExists => true, bCreateParent => true}); + + # Check if there are any files existing already. If none, that means a full copy is happening and we shouldn't report + # modified files + my @stryExistingList = $oStorageTest->list($strRepoCachePath, {bIgnoreMissing => true}); + + # First check if there is an old manifest that has not been cleared. This indicates that an error happened before all new + # files could be processed, which means they should be processed again. + my @stryModifiedList; + my $rstrModifiedList = $oStorageTest->get( + $oStorageTest->openRead("${strRepoCachePath}/${strRepoCacheManifest}", {bIgnoreMissing => true})); + + if (defined($rstrModifiedList)) + { + @stryModifiedList = split("\n", trim($$rstrModifiedList)); + } + + push( + @stryModifiedList, + split( + "\n", + trim( + executeTest( + "git -C ${strBackRestBase} ls-files -c --others --exclude-standard |" . + " rsync -rtW --out-format=\"\%n\" --delete --exclude=repo.manifest ${strBackRestBase}/" . + " --files-from=- ${strRepoCachePath}")))); + + if (@stryModifiedList > 0) + { + $oStorageTest->put("${strRepoCachePath}/${strRepoCacheManifest}", join("\n", @stryModifiedList)); + + if (@stryExistingList > 0) + { + &log(INFO, "modified since last run: " . join(', ', @stryModifiedList)); + } + } + # Generate code counts + #--------------------------------------------------------------------------------------------------------------------------- if ($bCodeCount) { &log(INFO, "classify code files"); @@ -371,15 +415,13 @@ eval #--------------------------------------------------------------------------------------------------------------------------- if (!$bNoGen) { + my @stryBuiltAll; &log(INFO, "check code autogenerate"); # Auto-generate C files #----------------------------------------------------------------------------------------------------------------------- - if (!$bSmart || buildLastModTime($oStorageBackRest, "${strBackRestBase}", ['build']) > - buildLastModTime($oStorageBackRest, "${strBackRestBase}", ['src'], '\.auto\.c$')) + if (!$bSmart || grep(/^build\//, @stryModifiedList)) { - &log(INFO, " autogenerate C code"); - errorDefineLoad(${$oStorageBackRest->get("build/error.yaml")}); my $rhBuild = @@ -409,7 +451,14 @@ eval }, }; - buildAll("${strBackRestBase}/src", $rhBuild); + my @stryBuilt = buildAll("${strBackRestBase}/src", $rhBuild); + &log(INFO, " autogenerated C code: " . (@stryBuilt ? join(', ', @stryBuilt) : 'no changes')); + + if (@stryBuilt) + { + push(@stryBuiltAll, @stryBuilt); + push(@stryModifiedList, @stryBuilt); + } } # Auto-generate Perl code @@ -417,23 +466,24 @@ eval use lib dirname(dirname($0)) . '/libc/build/lib'; use pgBackRestLibC::Build; ## no critic (Modules::ProhibitConditionalUseStatements) - if (!$bSmart || buildLastModTime($oStorageBackRest, "${strBackRestBase}", ['build', 'libc/build']) > - buildLastModTime($oStorageBackRest, "${strBackRestBase}", ['lib'], 'Auto\.pm$')) + if (!$bSmart || grep(/^(build|libc\/build)\//, @stryModifiedList)) { - &log(INFO, " autogenerate Perl code"); - errorDefineLoad(${$oStorageBackRest->get("build/error.yaml")}); - buildXsAll("${strBackRestBase}/libc"); + my @stryBuilt = buildXsAll("${strBackRestBase}/libc"); + &log(INFO, " autogenerated Perl code: " . (@stryBuilt ? join(', ', @stryBuilt) : 'no changes')); + + if (@stryBuilt) + { + push(@stryBuiltAll, @stryBuilt); + push(@stryModifiedList, @stryBuilt); + } } # Auto-generate C library code to embed in the binary #----------------------------------------------------------------------------------------------------------------------- - if (!$bSmart || buildLastModTime($oStorageBackRest, "${strBackRestBase}", ['libc']) > - buildLastModTime($oStorageBackRest, "${strBackRestBase}", ['src/perl'], '^libc\.auto\.c$')) + if (!$bSmart || grep(/^libc\//, @stryModifiedList)) { - &log(INFO, " autogenerate embedded C code"); - my $strLibC = executeTest( "cd ${strBackRestBase}/libc && " . "perl /usr/share/perl/5.26/ExtUtils/xsubpp -typemap /usr/share/perl/5.26/ExtUtils/typemap" . @@ -447,16 +497,23 @@ eval $strLibC =~ s/^\#line .*\n//mg; # Save into the bin src dir - $oStorageBackRest->put("${strBackRestBase}/src/perl/libc.auto.c", $strLibC); + my @stryBuilt; + my $strBuilt = 'src/perl/libc.auto.c'; + + if (buildPutDiffers($oStorageBackRest, "${strBackRestBase}/${strBuilt}", $strLibC)) + { + push(@stryBuilt, $strBuilt); + push(@stryBuiltAll, @stryBuilt); + push(@stryModifiedList, @stryBuilt); + } + + &log(INFO, " autogenerated embedded C code: " . (@stryBuilt ? join(', ', @stryBuilt) : 'no changes')); } # Auto-generate embedded Perl code #----------------------------------------------------------------------------------------------------------------------- - if (!$bSmart || buildLastModTime($oStorageBackRest, "${strBackRestBase}", ['lib']) > - buildLastModTime($oStorageBackRest, "${strBackRestBase}", ['src/perl'], 'embed\.auto\.c')) + if (!$bSmart || grep(/^lib\//, @stryModifiedList)) { - &log(INFO, " autogenerate embedded Perl code"); - my $rhBuild = { 'embed' => @@ -466,22 +523,44 @@ eval }, }; - buildAll("${strBackRestBase}/src", $rhBuild); + my @stryBuilt = buildAll("${strBackRestBase}/src", $rhBuild); + &log(INFO, " autogenerated embedded Perl code: " . (@stryBuilt ? join(', ', @stryBuilt) : 'no changes')); + + if (@stryBuilt) + { + push(@stryBuiltAll, @stryBuilt); + push(@stryModifiedList, @stryBuilt); + } } # Auto-generate C Makefile #----------------------------------------------------------------------------------------------------------------------- - if (!$bSmart || buildLastModTime($oStorageBackRest, "${strBackRestBase}", ['libc', 'src']) > - buildLastModTime($oStorageBackRest, "${strBackRestBase}", ['src'], 'Makefile')) + if (!$bSmart || grep(/^(src|libc)\//, @stryModifiedList)) { - &log(INFO, " autogenerate C Makefile"); + my @stryBuilt; + my $strBuilt = 'src/Makefile'; - $oStorageBackRest->put( - "src/Makefile", + if (buildPutDiffers( + $oStorageBackRest, + $strBuilt, buildMakefile( $oStorageBackRest, ${$oStorageBackRest->get("src/Makefile")}, - {rhOption => {'postgres/pageChecksum.o' => '-funroll-loops -ftree-vectorize'}})); + {rhOption => {'postgres/pageChecksum.o' => '-funroll-loops -ftree-vectorize'}}))) + { + push(@stryBuilt, $strBuilt); + push(@stryBuiltAll, @stryBuilt); + push(@stryModifiedList, @stryBuilt); + } + + &log(INFO, " autogenerated C Makefile: " . (@stryBuilt ? join(', ', @stryBuilt) : 'no changes')); + } + + # Copy all the files that were auto-generate so they won't show as modified in the next run + #----------------------------------------------------------------------------------------------------------------------- + foreach my $strBuilt (@stryBuiltAll) + { + executeTest("cp -p ${strBackRestBase}/${strBuilt} ${strRepoCachePath}/${strBuilt}"); } if ($bGenOnly) @@ -579,7 +658,7 @@ eval executeTest( "sudo rm -rf ${strTestPath}/cover_db ${strTestPath}/test-* ${strTestPath}/expect-*" . ($bDev ? '' : " ${strTestPath}/gcov-*")); - $oStorageTest->pathCreate($strCoveragePath, {strMode => '0770', bIgnoreMissing => true, bCreateParent => true}); + $oStorageTest->pathCreate($strCoveragePath, {strMode => '0770', bIgnoreExists => true, bCreateParent => true}); # Remove old coverage dirs -- do it this way so the dirs stay open in finder/explorer, etc. executeTest("rm -rf ${strBackRestBase}/test/coverage/c/* ${strBackRestBase}/test/coverage/perl/*"); @@ -1037,6 +1116,10 @@ eval exit 0 if $bBuildOnly; } + # Remove repo.manifest now that all processing that depends on modified files has been completed + #--------------------------------------------------------------------------------------------------------------------------- + $oStorageTest->remove("${strRepoCachePath}/${strRepoCacheManifest}"); + # Perform static source code analysis #--------------------------------------------------------------------------------------------------------------------------- if (!$bDryRun)