diff options
author | Ryan Keane <rwkeane@google.com> | 2020-06-24 17:44:18 -0700 |
---|---|---|
committer | Commit Bot <commit-bot@chromium.org> | 2020-06-25 15:46:13 +0000 |
commit | fdc66e03064d70f2c4a60dbd3fdd3925b21a43a1 (patch) | |
tree | c6bd7709e76d4bccf50be57f1a440e10423d76b0 | |
parent | dd834a700e0c201a5824682c6c020f3140df81d6 (diff) | |
download | openscreen-fdc66e03064d70f2c4a60dbd3fdd3925b21a43a1.tar.gz |
Code Coverage: Sync merge_steps.py up with Chromium
merge_steps.py in Chromium has updated. We need to copy the file to our
repo for code coverage to continue functioning. This CL performs that
copy.
See: https://source.chromium.org/chromium/chromium/src/+/master:testing/merge_scripts/code_coverage/merge_steps.py?q=merge_steps.py&ss=chromium&originalUrl=https:%2F%2Fcs.chromium.org%2F
Change-Id: I0bbe8e10481ffc51af59f1240094f1c4ceae36c5
Reviewed-on: https://chromium-review.googlesource.com/c/openscreen/+/2265095
Reviewed-by: Ryan Keane <rwkeane@google.com>
Commit-Queue: Ryan Keane <rwkeane@google.com>
-rw-r--r-- | build/code_coverage/merge_lib.py | 84 | ||||
-rw-r--r-- | build/code_coverage/merge_results.py | 30 | ||||
-rw-r--r-- | build/code_coverage/merge_steps.py | 21 | ||||
-rw-r--r-- | discovery/mdns/mdns_records.h | 6 |
4 files changed, 97 insertions, 44 deletions
diff --git a/build/code_coverage/merge_lib.py b/build/code_coverage/merge_lib.py index 818044c3..4b956d06 100644 --- a/build/code_coverage/merge_lib.py +++ b/build/code_coverage/merge_lib.py @@ -29,7 +29,7 @@ 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. Args: @@ -37,6 +37,8 @@ def _call_profdata_tool(profile_input_file_paths, are to be merged. profile_output_file_path: The path to the merged file to write. profdata_tool_path: The path to the llvm-profdata executable. + sparse (bool): flag to indicate whether to run llvm-profdata with --sparse. + Doc: https://llvm.org/docs/CommandGuide/llvm-profdata.html#profdata-merge Returns: A list of paths to profiles that had to be excluded to get the merge to @@ -45,12 +47,16 @@ def _call_profdata_tool(profile_input_file_paths, Raises: CalledProcessError: An error occurred merging profiles. """ + logging.debug('Profile input paths: %r' % profile_input_file_paths) + logging.debug('Profile output path: %r' % profile_output_file_path) try: subprocess_cmd = [ profdata_tool_path, 'merge', '-o', profile_output_file_path, - '-sparse=true' ] + if sparse: + subprocess_cmd += ['-sparse=true',] subprocess_cmd.extend(profile_input_file_paths) + logging.info('profdata command: %r', ' '.join(subprocess_cmd)) # Redirecting stderr is required because when error happens, llvm-profdata # writes the error output to stderr and our error handling logic relies on @@ -58,34 +64,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 @@ -108,7 +86,9 @@ def _get_profile_paths(input_dir, return paths -def _validate_and_convert_profraws(profraw_files, profdata_tool_path): +def _validate_and_convert_profraws(profraw_files, + profdata_tool_path, + sparse=True): """Validates and converts profraws to profdatas. For each given .profraw file in the input, this method first validates it by @@ -121,6 +101,8 @@ def _validate_and_convert_profraws(profraw_files, profdata_tool_path): Args: profraw_files: A list of .profraw paths. profdata_tool_path: The path to the llvm-profdata executable. + sparse (bool): flag to indicate whether to run llvm-profdata with --sparse. + Doc: https://llvm.org/docs/CommandGuide/llvm-profdata.html#profdata-merge Returns: A tulple: @@ -144,7 +126,7 @@ def _validate_and_convert_profraws(profraw_files, profdata_tool_path): pool.apply_async( _validate_and_convert_profraw, (profraw_file, output_profdata_files, invalid_profraw_files, - counter_overflows, profdata_tool_path)) + counter_overflows, profdata_tool_path, sparse)) pool.close() pool.join() @@ -159,16 +141,25 @@ def _validate_and_convert_profraws(profraw_files, profdata_tool_path): def _validate_and_convert_profraw(profraw_file, output_profdata_files, invalid_profraw_files, counter_overflows, - profdata_tool_path): + profdata_tool_path, sparse=True): output_profdata_file = profraw_file.replace('.profraw', '.profdata') subprocess_cmd = [ - profdata_tool_path, 'merge', '-o', output_profdata_file, '-sparse=true', - profraw_file + profdata_tool_path, + 'merge', + '-o', + output_profdata_file, ] + if sparse: + subprocess_cmd.append('--sparse') + + subprocess_cmd.append(profraw_file) + profile_valid = False counter_overflow = False validation_output = None + logging.info('profdata command: %r', ' '.join(subprocess_cmd)) + # 1. Determine if the profile is valid. try: # Redirecting stderr is required because when error happens, llvm-profdata @@ -237,7 +228,9 @@ def merge_profiles(input_dir, output_file, input_extension, profdata_tool_path, - input_filename_pattern='.*'): + input_filename_pattern='.*', + sparse=True, + skip_validation=False): """Merges the profiles produced by the shards using llvm-profdata. Args: @@ -248,6 +241,11 @@ def merge_profiles(input_dir, profdata_tool_path: The path to the llvm-profdata executable. input_filename_pattern (str): The regex pattern of input filename. Should be 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 succeed and a list of profiles that had a counter overflow. @@ -257,10 +255,16 @@ 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)) + profdata_tool_path, + sparse=sparse)) logging.info('List of converted .profdata files: %r', profile_input_file_paths) logging.info(( @@ -284,7 +288,8 @@ def merge_profiles(input_dir, invalid_profdata_files = _call_profdata_tool( profile_input_file_paths=profile_input_file_paths, profile_output_file_path=output_file, - profdata_tool_path=profdata_tool_path) + profdata_tool_path=profdata_tool_path, + sparse=sparse) # Remove inputs when merging profraws as they won't be needed and they can be # pretty large. If the inputs are profdata files, do not remove them as they @@ -320,3 +325,4 @@ def get_shards_to_retry(bad_profiles): assert is_task_id(task_id) bad_shard_ids.add(task_id) return bad_shard_ids + diff --git a/build/code_coverage/merge_results.py b/build/code_coverage/merge_results.py index a049a906..67e63365 100644 --- a/build/code_coverage/merge_results.py +++ b/build/code_coverage/merge_results.py @@ -35,6 +35,32 @@ def _MergeAPIArgumentParser(*args, **kwargs): '--per-cl-coverage', action='store_true', help='set to indicate that this is a per-CL coverage build') + # TODO(crbug.com/1077304) - migrate this to sparse=False as default, and have + # --sparse to set sparse + parser.add_argument( + '--no-sparse', + action='store_false', + dest='sparse', + help='run llvm-profdata without the sparse flag.') + # TODO(crbug.com/1077304) - The intended behaviour is to default sparse to + # false. --no-sparse above was added as a workaround, and will be removed. + # This is being introduced now in support of the migration to intended + # behavior. Ordering of args matters here, as the default is set by the former + # (sparse defaults to False because of ordering. See unit tests for details) + parser.add_argument( + '--sparse', + 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 @@ -47,7 +73,9 @@ def main(): invalid_profiles, counter_overflows = coverage_merger.merge_profiles( params.task_output_dir, os.path.join(params.profdata_dir, output_prodata_filename), '.profraw', - params.llvm_profdata) + params.llvm_profdata, + 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/build/code_coverage/merge_steps.py b/build/code_coverage/merge_steps.py index 40d0e946..f1140938 100644 --- a/build/code_coverage/merge_steps.py +++ b/build/code_coverage/merge_steps.py @@ -28,6 +28,24 @@ def _merge_steps_argument_parser(*args, **kwargs): default='.*', help='regex pattern of profdata filename to merge for current test type. ' 'If not present, all profdata files will be merged.') + # TODO(crbug.com/1077304) - migrate this to sparse=False as default, and have + # --sparse to set sparse + parser.add_argument( + '--no-sparse', + action='store_false', + dest='sparse', + help='run llvm-profdata without the sparse flag.') + # TODO(crbug.com/1077304) - The intended behaviour is to default sparse to + # false. --no-sparse above was added as a workaround, and will be removed. + # This is being introduced now in support of the migration to intended + # behavior. Ordering of args matters here, as the default is set by the former + # (sparse defaults to False because of ordering. See merge_results unit tests + # for details) + parser.add_argument( + '--sparse', + action='store_true', + dest='sparse', + help='run llvm-profdata with the sparse flag.') return parser @@ -36,7 +54,8 @@ def main(): parser = _merge_steps_argument_parser(description=desc) params = parser.parse_args() merger.merge_profiles(params.input_dir, params.output_file, '.profdata', - params.llvm_profdata, params.profdata_filename_pattern) + params.llvm_profdata, params.profdata_filename_pattern, + sparse=params.sparse) if __name__ == '__main__': diff --git a/discovery/mdns/mdns_records.h b/discovery/mdns/mdns_records.h index 27efdc66..9212b519 100644 --- a/discovery/mdns/mdns_records.h +++ b/discovery/mdns/mdns_records.h @@ -135,9 +135,9 @@ class RawRecordRdata { }; // SRV record format (http://www.ietf.org/rfc/rfc2782.txt): -// 2 bytes network-order unsigned priority -// 2 bytes network-order unsigned weight -// 2 bytes network-order unsigned port +// 2 bytes network-order unsigned priority +// 2 bytes network-order unsigned weight +// 2 bytes network-order unsigned port // target: domain name (on-the-wire representation) class SrvRecordRdata { public: |