Re: [PR] perf: Cache Transaction.table_metadata between staged updates [iceberg-python]

["rynewang (via GitHub)" <gi...@apache.org>]

rynewang commented on PR #3302:
URL: https://github.com/apache/iceberg-python/pull/3302#issuecomment-4357169100

   Hi @geruh thanks for the fast review!
   
   On the simplified design though, the case it doesn't cover is `CreateTableTransaction: _initial_changes()` which seeds `self._updates` with ~8–10 entries (`AssignUUID, UpgradeFormatVersion, AddSchema, SetCurrentSchema, AddPartitionSpec, SetDefaultSpec, AddSortOrder, SetDefaultSortOrder, SetLocation, SetProperties`) right in `__init__`, so `_updates` is never empty for the snapshot producer's whole lifetime. Every table_metadata read during `_summary()` / `_manifests()` / the new_manifest_writer/spec/schema helpers would not be benefited from the cache. That's the path I was profiling — create+append in one transaction, which is where the repeated deep-copy cost actually bites.
   
   Same story for any multi-op transaction where something stages an update before the append (e.g. set_properties then fast_append).
   
   My use case is: create a `CreateTableTransaction`, add a lot of parquets then call commit; this path calls _summary and _manifests and would be hit.
   
   Keeping a ref to the update tuple wont copy the memory, it just increments a refcount so I think it's fine?


-- 
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: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org