diff options
Diffstat (limited to 'chromium/testing/merge_scripts/code_coverage')
4 files changed, 64 insertions, 90 deletions
diff --git a/chromium/testing/merge_scripts/code_coverage/merge_lib.py b/chromium/testing/merge_scripts/code_coverage/merge_lib.py index 92851121635..4b6cda35cc3 100644 --- a/chromium/testing/merge_scripts/code_coverage/merge_lib.py +++ b/chromium/testing/merge_scripts/code_coverage/merge_lib.py @@ -23,7 +23,6 @@ logging.basicConfig( def _call_profdata_tool(profile_input_file_paths, profile_output_file_path, profdata_tool_path, - retries=3, sparse=True): """Calls the llvm-profdata tool. @@ -59,34 +58,6 @@ def _call_profdata_tool(profile_input_file_paths, output = subprocess.check_output(subprocess_cmd, stderr=subprocess.STDOUT) logging.info('Merge succeeded with output: %r', output) except subprocess.CalledProcessError as error: - if len(profile_input_file_paths) > 1 and retries >= 0: - logging.warning('Merge failed with error output: %r', error.output) - - # The output of the llvm-profdata command will include the path of - # malformed files, such as - # `error: /.../default.profraw: Malformed instrumentation profile data` - invalid_profiles = [ - f for f in profile_input_file_paths if f in error.output - ] - - if not invalid_profiles: - logging.info( - 'Merge failed, but wasn\'t able to figure out the culprit invalid ' - 'profiles from the output, so skip retry and bail out.') - raise error - - valid_profiles = list( - set(profile_input_file_paths) - set(invalid_profiles)) - if valid_profiles: - logging.warning( - 'Following invalid profiles are removed as they were mentioned in ' - 'the merge error output: %r', invalid_profiles) - logging.info('Retry merging with the remaining profiles: %r', - valid_profiles) - return invalid_profiles + _call_profdata_tool( - valid_profiles, profile_output_file_path, profdata_tool_path, - retries - 1) - logging.error('Failed to merge profiles, return code (%d), output: %r' % (error.returncode, error.output)) raise error @@ -252,7 +223,8 @@ def merge_profiles(input_dir, input_extension, profdata_tool_path, input_filename_pattern='.*', - sparse=True): + sparse=True, + skip_validation=False): """Merges the profiles produced by the shards using llvm-profdata. Args: @@ -265,6 +237,8 @@ def merge_profiles(input_dir, a valid regex pattern if present. sparse (bool): flag to indicate whether to run llvm-profdata with --sparse. Doc: https://llvm.org/docs/CommandGuide/llvm-profdata.html#profdata-merge + skip_validation (bool): flag to skip the _validate_and_convert_profraws + invocation. only applicable when input_extension is .profraw. Returns: The list of profiles that had to be excluded to get the merge to @@ -275,7 +249,12 @@ def merge_profiles(input_dir, input_filename_pattern) invalid_profraw_files = [] counter_overflows = [] - if input_extension == '.profraw': + + if skip_validation: + logging.warning('--skip-validation has been enabled. Skipping conversion ' + 'to ensure that profiles are valid.') + + if input_extension == '.profraw' and not skip_validation: profile_input_file_paths, invalid_profraw_files, counter_overflows = ( _validate_and_convert_profraws(profile_input_file_paths, profdata_tool_path, diff --git a/chromium/testing/merge_scripts/code_coverage/merge_lib_test.py b/chromium/testing/merge_scripts/code_coverage/merge_lib_test.py index 9da452e9c0c..97c1fbefb3a 100755 --- a/chromium/testing/merge_scripts/code_coverage/merge_lib_test.py +++ b/chromium/testing/merge_scripts/code_coverage/merge_lib_test.py @@ -8,13 +8,6 @@ import subprocess import sys import unittest -THIS_DIR = os.path.dirname(os.path.abspath(__file__)) -sys.path.insert( - 0, - os.path.abspath( - os.path.join(THIS_DIR, os.pardir, os.pardir, os.pardir, 'third_party', - 'pymock'))) - import mock import merge_lib as merger diff --git a/chromium/testing/merge_scripts/code_coverage/merge_results.py b/chromium/testing/merge_scripts/code_coverage/merge_results.py index d8ea2e098aa..199464b1dbc 100755 --- a/chromium/testing/merge_scripts/code_coverage/merge_results.py +++ b/chromium/testing/merge_scripts/code_coverage/merge_results.py @@ -72,6 +72,15 @@ def _MergeAPIArgumentParser(*args, **kwargs): action='store_true', dest='sparse', help='run llvm-profdata with the sparse flag.') + # (crbug.com/1091310) - IR PGO is incompatible with the initial conversion + # of .profraw -> .profdata that's run to detect validation errors. + # Introducing a bypass flag that'll merge all .profraw directly to .profdata + parser.add_argument( + '--skip-validation', + action='store_true', + help='skip validation for good raw profile data. this will pass all ' + 'raw profiles found to llvm-profdata to be merged. only applicable ' + 'when input extension is .profraw.') return parser @@ -106,7 +115,8 @@ def main(): params.task_output_dir, os.path.join(params.profdata_dir, output_prodata_filename), '.profraw', params.llvm_profdata, - sparse=params.sparse) + sparse=params.sparse, + skip_validation=params.skip_validation) # At the moment counter overflows overlap with invalid profiles, but this is # not guaranteed to remain the case indefinitely. To avoid future conflicts diff --git a/chromium/testing/merge_scripts/code_coverage/merge_results_test.py b/chromium/testing/merge_scripts/code_coverage/merge_results_test.py index 2af89a62881..8094582d8d1 100755 --- a/chromium/testing/merge_scripts/code_coverage/merge_results_test.py +++ b/chromium/testing/merge_scripts/code_coverage/merge_results_test.py @@ -10,11 +10,6 @@ import subprocess import sys import unittest -THIS_DIR = os.path.dirname(os.path.abspath(__file__)) -sys.path.insert( - 0, os.path.abspath(os.path.join(THIS_DIR, os.pardir, os.pardir, os.pardir, - 'third_party', 'pymock'))) - import mock import merge_results @@ -55,7 +50,8 @@ class MergeProfilesTest(unittest.TestCase): self.assertEqual( mock_merge.call_args, mock.call(task_output_dir, profdata_file, '.profraw', - 'llvm-profdata', sparse=True), None) + 'llvm-profdata', sparse=True, + skip_validation=False), None) def test_merge_steps_parameters(self): """Test the build-level merge front-end.""" @@ -123,6 +119,45 @@ class MergeProfilesTest(unittest.TestCase): self.assertTrue(mock_validate_and_convert_profraws.called) + @mock.patch.object(merger, '_validate_and_convert_profraws') + def test_profraw_skip_validation(self, mock_validate_and_convert_profraws): + mock_input_dir_walk = [ + ('/b/some/path', ['0', '1', '2', '3'], ['summary.json']), + ('/b/some/path/0', [], + ['output.json', 'default-1.profraw', 'default-2.profraw']), + ('/b/some/path/1', [], + ['output.json', 'default-1.profraw', 'default-2.profraw']), + ] + + with mock.patch.object(os, 'walk') as mock_walk: + with mock.patch.object(os, 'remove'): + mock_walk.return_value = mock_input_dir_walk + with mock.patch.object(subprocess, 'check_output') as mock_exec_cmd: + merger.merge_profiles('/b/some/path', + 'output/dir/default.profdata', + '.profraw', + 'llvm-profdata', + skip_validation=True) + self.assertEqual( + mock.call( + [ + 'llvm-profdata', + 'merge', + '-o', + 'output/dir/default.profdata', + '-sparse=true', + '/b/some/path/0/default-1.profraw', + '/b/some/path/0/default-2.profraw', + '/b/some/path/1/default-1.profraw', + '/b/some/path/1/default-2.profraw' + ], + stderr=-2, + ), mock_exec_cmd.call_args) + + # Skip validation should've passed all profraw files directly, and + # this validate call should not have been invoked. + self.assertFalse(mock_validate_and_convert_profraws.called) + def test_merge_profraw_skip_if_there_is_no_file(self): mock_input_dir_walk = [ @@ -206,50 +241,6 @@ class MergeProfilesTest(unittest.TestCase): # The mock method should only apply when merging .profraw files. self.assertFalse(mock_validate_and_convert_profraws.called) - def test_retry_profdata_merge_failures(self): - mock_input_dir_walk = [ - ('/b/some/path', ['0', '1'], ['summary.json']), - ('/b/some/path/0', [], - ['output.json', 'default-1.profdata', 'default-2.profdata']), - ('/b/some/path/1', [], - ['output.json', 'default-1.profdata', 'default-2.profdata']), - ] - with mock.patch.object(os, 'walk') as mock_walk: - with mock.patch.object(os, 'remove'): - mock_walk.return_value = mock_input_dir_walk - with mock.patch.object(subprocess, 'check_output') as mock_exec_cmd: - invalid_profiles_msg = ( - 'error: /b/some/path/0/default-1.profdata: Malformed ' - 'instrumentation profile data.') - - # Failed on the first merge, but succeed on the second attempt. - mock_exec_cmd.side_effect = [ - subprocess.CalledProcessError( - returncode=1, cmd='dummy cmd', output=invalid_profiles_msg), - None - ] - - merger.merge_profiles('/b/some/path', 'output/dir/default.profdata', - '.profdata', 'llvm-profdata') - - self.assertEqual(2, mock_exec_cmd.call_count) - - # Note that in the second call, /b/some/path/0/default-1.profdata is - # excluded! - self.assertEqual( - mock.call( - [ - 'llvm-profdata', - 'merge', - '-o', - 'output/dir/default.profdata', - '-sparse=true', - '/b/some/path/0/default-2.profdata', - '/b/some/path/1/default-1.profdata', - '/b/some/path/1/default-2.profdata', - ], - stderr=-2, - ), mock_exec_cmd.call_args) @mock.patch('os.remove') def test_mark_invalid_shards(self, mock_rm): @@ -380,7 +371,8 @@ class MergeProfilesTest(unittest.TestCase): self.assertEqual( mock_merge.call_args, mock.call(task_output_dir, profdata_file, '.profraw', - 'llvm-profdata', sparse=expected_outcome), None) + 'llvm-profdata', sparse=expected_outcome, + skip_validation=False), None) if __name__ == '__main__': |