Skip to content

Commit d48dc52

Browse files
authored
Fix some bugs in upload benchmark scripts (#6429)
While working on #6425, I discover several bugs in the upload scripts: * If there is an invalid JSON file in the directory, the script returns instead of continue, skipping all records after. Covered by https://github.com/pytorch/test-infra/blob/main/.github/scripts/benchmark-results-dir-for-testing/v3/mock.json * The script didn't handle correctly JSONEachRow format with only one record. Covered by a new test JSON from https://github.com/pytorch/test-infra/pull/6425/files#diff-bff954994eb33173b7119ff8d280f3367117b2daa9b8c54888be5f48f183a280 * The script didn't handle correctly JSONEachRow format mix with list of records. Covered by https://github.com/pytorch/test-infra/blob/main/.github/scripts/benchmark-results-dir-for-testing/v3/json-each-row.json#L3 ### Testing https://github.com/pytorch/test-infra/actions/runs/13909203687/job/38919334944#step:5:125 looks correct now
1 parent 883310b commit d48dc52

File tree

3 files changed

+29
-13
lines changed

3 files changed

+29
-13
lines changed
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
[{"benchmark": {"name": "pr_time_benchmarks", "extra_info": {"is_dynamic": false, "device": "cpu", "description": "a loop over 100 add node"}}, "model": {"name": "add_loop_eager", "type": "add_loop", "backend": "eager"}, "metric": {"name": "compile_time_instruction_count", "benchmark_values": [3086359081]}}]
1+
[{"benchmark": {"name": "pr_time_benchmarks", "extra_info": {"is_dynamic": false, "device": "cpu", "description": "a loop over 100 add node"}}, "model": {"name": "add_loop_eager", "type": "add_loop", "backend": "eager"}, "metric": {"name": "compile_time_instruction_count", "benchmark_values": [3086359081]}}]
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
[{"benchmark": {"name": "pr_time_benchmarks", "extra_info": {"is_dynamic": true, "device": "cuda", "description": "a loop over 100 add node"}}, "model": {"name": "add_loop_inductor_dynamic_gpu", "type": "add_loop", "backend": "inductor"}, "metric": {"name": "compile_time_instruction_count", "benchmark_values": [40859830085]}}]
1+
[{"benchmark": {"name": "pr_time_benchmarks", "extra_info": {"is_dynamic": true, "device": "cuda", "description": "a loop over 100 add node"}}, "model": {"name": "add_loop_inductor_dynamic_gpu", "type": "add_loop", "backend": "inductor"}, "metric": {"name": "compile_time_instruction_count", "benchmark_values": [40859830085]}}]

.github/scripts/upload_benchmark_results.py

+27-11
Original file line numberDiff line numberDiff line change
@@ -172,7 +172,8 @@ def upload_to_dynamodb(
172172
"""
173173
Copied from upload stats script
174174
"""
175-
info(f"Writing {len(docs)} documents to DynamoDB {dynamodb_table}")
175+
msg = f"Writing {len(docs)} documents to DynamoDB {dynamodb_table}"
176+
info(msg)
176177
if not dry_run:
177178
# https://boto3.amazonaws.com/v1/documentation/api/latest/guide/dynamodb.html#batch-writing
178179
with boto3.resource("dynamodb").Table(dynamodb_table).batch_writer() as batch:
@@ -187,19 +188,31 @@ def read_benchmark_results(filepath: str) -> List[Dict[str, Any]]:
187188
benchmark_results = []
188189
with open(filepath) as f:
189190
try:
190-
benchmark_results = json.load(f)
191+
r = json.load(f)
192+
# Handle the JSONEachRow case where there is only one record in the
193+
# JSON file, it can still be loaded normally, but will need to be
194+
# added into the list of benchmark results with the length of 1
195+
if isinstance(r, dict):
196+
benchmark_results.append(r)
197+
elif isinstance(r, list):
198+
benchmark_results = r
199+
191200
except JSONDecodeError:
192201
f.seek(0)
193202

194203
# Try again in ClickHouse JSONEachRow format
195204
for line in f:
196205
try:
197206
r = json.loads(line)
198-
# Each row needs to be a dictionary in JSON format
199-
if not isinstance(r, dict):
200-
warn(f"Not a JSON dict {line}, skipping")
207+
# Each row needs to be a dictionary in JSON format or a list
208+
if isinstance(r, dict):
209+
benchmark_results.append(r)
210+
elif isinstance(r, list):
211+
benchmark_results.extend(r)
212+
else:
213+
warn(f"Not a JSON dict or list {line}, skipping")
201214
continue
202-
benchmark_results.append(r)
215+
203216
except JSONDecodeError:
204217
warn(f"Invalid JSON {line}, skipping")
205218

@@ -220,7 +233,7 @@ def process_benchmark_results(
220233
for result in benchmark_results:
221234
# This is a required field
222235
if "metric" not in result:
223-
warn(f"{result} is not a benchmark record, skipping")
236+
warn(f"{result} from {filepath} is not a benchmark record, skipping")
224237
continue
225238

226239
record: Dict[str, Any] = {**metadata, **result}
@@ -284,10 +297,12 @@ def upload_to_s3(
284297
"""
285298
s3_path = generate_s3_path(benchmark_results, filepath, schema_version)
286299
if not s3_path:
287-
info(f"Could not generate an S3 path for {filepath}, skipping...")
300+
msg = f"Could not generate an S3 path for {filepath}, skipping..."
301+
info(msg)
288302
return
289303

290-
info(f"Upload {filepath} to s3://{s3_bucket}/{s3_path}")
304+
msg = f"Upload {filepath} to s3://{s3_bucket}/{s3_path}"
305+
info(msg)
291306
if not dry_run:
292307
# Write in JSONEachRow format
293308
data = "\n".join([json.dumps(result) for result in benchmark_results])
@@ -314,7 +329,8 @@ def main() -> None:
314329
# NB: This is for backward compatibility before we move to schema v3
315330
if schema_version == "v2":
316331
with open(filepath) as f:
317-
info(f"Uploading {filepath} to dynamoDB ({schema_version})")
332+
msg = f"Uploading {filepath} to dynamoDB ({schema_version})"
333+
info(msg)
318334
upload_to_dynamodb(
319335
dynamodb_table=args.dynamodb_table,
320336
# NB: DynamoDB only accepts decimal number, not float
@@ -331,7 +347,7 @@ def main() -> None:
331347
)
332348

333349
if not benchmark_results:
334-
return
350+
continue
335351

336352
upload_to_s3(
337353
s3_bucket=OSSCI_BENCHMARKS_BUCKET,

0 commit comments

Comments
 (0)