Ma77Ball commented on code in PR #9008:
URL: https://github.com/apache/texera/pull/9008#discussion_r4291042088
##########
amber/src/main/python/core/models/internal_queue.py:
##########
@@ -55,13 +56,24 @@ class ECMElement(InternalQueueElement):
T = TypeVar("T", bound=InternalQueueElement)
+def _in_mem_size(item) -> int:
+ """
+ Bytes an item holds while queued. The total is reported to the JVM as
+ queued credit for flow control, so a data frame counts its Arrow buffers
+ rather than the few bytes of its Python wrapper.
+ """
+ if isinstance(item, DataElement) and isinstance(item.payload, DataFrame):
+ return item.payload.frame.get_total_buffer_size()
+ return sys.getsizeof(item)
Review Comment:
`StateFrame`, the other `DataPayload` subclass, falls through to
`sys.getsizeof` here, which measures only the ~48-byte `DataElement` wrapper,
not the `State` dict it carries (and `State` content can hold base64-encoded
values). That is the same under-counting this PR fixes for `DataFrame`.
This is pre-existing (before this PR every item was sized by
`sys.getsizeof`) and may well be intentional if states are always small, so it
does not block. Worth either sizing `StateFrame` too in a follow-up or
confirming the content stays small. What is the expected upper bound on a
`State` payload?
--
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]
