damccorm commented on PR #41060:
URL: https://github.com/apache/beam/pull/41060#issuecomment-6779178215
> On the description's Avro numbers: running the linked `bench_45` body
unchanged, I get 245 ms → 111 ms (2.2x) and the output going from 1,080,252 to
540,966 bytes (2.0x), not 16x and 12.2x. Most of that gain is the regex change,
since `bench_45` calls `_split_table_spec` 30,000 times and that call goes from
5.25 µs to 1.6 µs. Were the 16x and 12.2x from a different version of the
benchmark?
>
> Repro
> Python 3.13.9, fastavro 1.12.2, regex 2026.6.28. Run from `sdks/python` on
master and on this branch.
>
> ```
> python -m timeit -s "from apache_beam.io.gcp.bigquery_tools import
_split_table_spec" "_split_table_spec('my-project:my_dataset.my_table')"
> ```
>
> ```python
> # body of bench_45_bigquery_tools_avro_and_json at c2feb961eef, unchanged
> import io, time
> from apache_beam.io.gcp import bigquery_tools
>
> def bench_45():
> schema_dict = {'fields': [{'name': 'k', 'type': 'STRING', 'mode':
'NULLABLE'},
> {'name': 'v', 'type': 'INTEGER', 'mode':
'NULLABLE'}]}
> bio = io.BytesIO()
> writer = bigquery_tools.AvroRowWriter(bio, schema_dict)
> row = {'k': 'user_12345', 'v': 987654321}
> t0 = time.perf_counter()
> for _ in range(30000):
> writer.write(row)
> _ = writer.tell()
> _ = bigquery_tools._split_table_spec('my-project:my_dataset.my_table')
> return time.perf_counter() - t0, writer
>
> times = []
> for _ in range(7):
> t, writer = bench_45()
> times.append(t)
> writer.flush()
> print('best %.1f ms, %d bytes' % (min(times) * 1e3,
writer._file_handle.tell()))
> ```
Thanks for calling this out. You're right that this pulled in the wrong
number (it grabbed a different metric from the summary doc I generated).
I'll try to do a better job of reviewing the description for correctness
moving forward as well, I had focused strictly on the contents and directional
correctness of the benchmark, but I agree that the magnitude matters too.
Btw, as I took one more pass, I also noticed that the `decode` change was
actually moving things in the wrong direction (though it didn't super
meaningfully influence the overall benchmark), so I removed that. I did find
that the other changes were all positively contributing though.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
