tvalentyn commented on code in PR #41159: URL: https://github.com/apache/beam/pull/41159#discussion_r4531658158
########## sdks/python/apache_beam/runners/common.py: ########## @@ -65,6 +65,7 @@ from apache_beam.utils.windowed_value import HomogeneousWindowedBatch from apache_beam.utils.windowed_value import WindowedBatch from apache_beam.utils.windowed_value import WindowedValue +from apache_beam.utils.windowed_value import create as windowed_value_create Review Comment: agents tell me it would be advantageous to repeat this import in common.pxd . Could you check if the claim holds up? Missed C-level function dispatch for windowed_value_create Location: [sdks/python/apache_beam/runners/common.py#L68](https://github.com/apache/beam/blob/dfa529d59000931b380ae2e0c1766f858fc16071/sdks/python/apache_beam/runners/common.py#L68) and [sdks/python/apache_beam/runners/common.pxd#L20-L22](https://github.com/apache/beam/blob/dfa529d59000931b380ae2e0c1766f858fc16071/sdks/python/apache_beam/runners/common.pxd#L20-L22) What is wrong: common.py imports create as windowed_value_create via standard Python import, but common.pxd does not cimport create from windowed_value.pxd (where it is declared as cpdef WindowedValue create(object value, int64_t timestamp_micros, object windows, object pane_info=*) at [windowed_value.pxd#L54-L56](https://github.com/apache/beam/blob/dfa529d59000931b380ae2e0c1766f858fc16071/sdks/python/apache_beam/utils/windowed_value.pxd#L54-L56)). Consequently, Cython calls windowed_value_create via PyObject_Call / Python dynamic call rather than a direct C function pointer call across extension modules. Why it matters: On multi-window elements where windows are exploded, calling windowed_value_create still incurs Python call overhead instead of running at pure C speed. Suggested fix: In common.pxd, add from apache_beam.utils.windowed_value cimport create as windowed_value_create (or from apache_beam.utils cimport windowed_value and call windowed_value.create(...) in common.py, identical to how [coder_impl.pxd#L33](https://github.com/apache/beam/blob/dfa529d59000931b380ae2e0c1766f858fc16071/sdks/python/apache_beam/coders/coder_impl.pxd#L33) does it). -- 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]
