claudevdm commented on code in PR #41060:
URL: https://github.com/apache/beam/pull/41060#discussion_r4530658170
##########
sdks/python/apache_beam/io/gcp/bigquery_tools.py:
##########
@@ -1562,9 +1570,15 @@ def insert_rows(
str(self.unique_row_id) if not insert_ids else insert_ids[i]
for i, _ in enumerate(rows)
]
+ _json_safe = (str, bool, type(None))
rows = [
- fast_json_loads(fast_json_dumps(r, default=default_encoder))
- for r in rows
+ r if (
Review Comment:
This check wins when a row is mostly long strings, but with orjson installed
(the `gcp` extra requires it) it is slower than the round trip for short
values, and a row that fails the check pays for both. Timing `insert_rows` with
`_insert_all_rows` stubbed out (Python 3.13.9, orjson 3.11.9, ns per row, at
5aeb8d3d7ca):
| 10-column row | master | this PR |
|---|---|---|
| 5 short str + 5 int | 643 | 1046 |
| 10 int | 666 | 1337 |
| 5 short str + 4 int + 1 float | 657 | 1878 |
| 10 str × 10 chars | 790 | 700 |
| 10 str × 100 chars | 1056 | 707 |
| 10 str × 1000 chars | 6409 | 698 |
| 10 str × 1000 chars + 1 float | 4421 | 5316 |
So any row with a FLOAT, RECORD or REPEATED column gets slower, and so do
rows of short ints and strings. What row was the description's 5.52x measured
on, and was orjson installed? Its 116.9 ms for 30,000 rows is 3.9 µs per row on
master, and I only get near that with orjson for rows of roughly 1 kB strings.
If the target is string-heavy rows, is the slowdown on the other shapes
acceptable?
<details><summary>Repro</summary>
Run from `sdks/python` on master and on this branch.
```python
import timeit
from apache_beam.io.gcp import bigquery_tools as bt
w = object.__new__(bt.BigQueryWrapper)
w._insert_all_rows = lambda *a, **k: (True, [])
def ns_per_row(row):
rows = [dict(row) for _ in range(500)]
ids = ['i'] * 500
run = lambda: w.insert_rows('p', 'd', 't', rows, insert_ids=ids)
return min(timeit.repeat(run, number=10, repeat=9)) / 10 / 500 * 1e9
def strs(length):
return {'c%d' % i: str(i) + 'x' * (length - 1) for i in range(10)}
mixed = {'c%d' % i: ('v%d' % i if i % 2 else i) for i in range(10)}
rows = {
'5 short str + 5 int': mixed,
'10 int': {'c%d' % i: i * 1000 for i in range(10)},
'5 short str + 4 int + 1 float': {**mixed, 'c9': 1.5},
'10 str x 10 chars': strs(10),
'10 str x 100 chars': strs(100),
'10 str x 1000 chars': strs(1000),
'10 str x 1000 chars + 1 float': {**strs(1000), 'f': 1.5},
}
print('json lib:', bt.fast_json_dumps.__module__)
for name, row in rows.items():
print('%-30s %6.0f ns/row' % (name, ns_per_row(row)))
```
Surrogate case (master: `TypeError: str is not valid UTF-8: surrogates not
allowed`; this branch: prints `sent: [{'a': '\ud800'}]`):
```python
from apache_beam.io.gcp import bigquery_tools as bt
w = object.__new__(bt.BigQueryWrapper)
w._insert_all_rows = lambda p, d, t, rows, ids, **k: print('sent:', rows) or
(True, [])
w.insert_rows('p', 'd', 't', [{'a': '\ud800'}], insert_ids=['i'])
```
</details>
--
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]
