Re: [PR] [SPARK-57521][ML][CORE] Add skipTransient mode to SizeEstimator to fix ML cache overcounting [spark]

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

mkincaid commented on PR #56584:
URL: https://github.com/apache/spark/pull/56584#issuecomment-4999347328

   Hi @uros-b and @zhengruifeng, thanks for the comments and thoughtful review. @zhengruifeng's response on the design options persuaded me that it would be cleaner to address this in SizeEstimator than the model, but reflecting further, we should try to find a fully general way to make the relevant distinction between what we care about counting and what we don't, rather than add another special case for `SparkSession`/`SparkContext` objects. That distinction, it seems to me, is `@transient`: transient references are by definition not data that intrinsically needs to stay with the object (that's why they don't get serialized), and in the context of considering eviction from the ML cache, we're asking how much memory would be reclaimed by freeing the object (nothing transient, because that's all shared state that doesn't belong exclusively to the object).
   
   So in this version of the patch, I've added a `skipTransient` option to `SizeEstimator` which will skip `@transient` pointer fields during the graph traversal. This does now expand the scope to the core, but `skipTransient` is false by default, so this should be a safe change that doesn't affect any other callers; only `Model.estimatedSize` opts into using it. Interested to hear your thoughts on this approach.


-- 
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: reviews-unsubscribe@spark.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org