You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by GitBox <gi...@apache.org> on 2021/08/19 16:44:56 UTC

[GitHub] [spark] zsxwing edited a comment on pull request #33784: Revert "[SPARK-34309][BUILD][CORE][SQL][K8S] Use Caffeine instead of Guava Cache"

zsxwing edited a comment on pull request #33784:
URL: https://github.com/apache/spark/pull/33784#issuecomment-902073060


   @LuciferYang Thank you for submitting the revert PR.
   
   @sarutak  @dongjoon-hyun As I mentioned in [the previous PR](https://github.com/apache/spark/pull/31517#issuecomment-897405632), adding a popular library to Spark's classpath directly is risky. @LuciferYang 's [suggestion](https://github.com/apache/spark/pull/31517#issuecomment-897430503) to shade Caffeine is a good solution. But it sounds not worth because:
   - There is no evidence to show performance improvement in practice. The code path using cache is not on any hot path, and the difference between Guava Cache and Caffeine is probably not noticeable in practice.
   - Extra work to shade the following three libraries (Need to verify this approach is working): 
   ```
   caffeine/2.9.2//caffeine-2.9.2.jar
   checker-qual/3.10.0//checker-qual-3.10.0.jar
   error_prone_annotations/2.5.1//error_prone_annotations-2.5.1.jar
   ```
   - Increase the maintenance burden when backporting a patch that touches the changed code.
   
   Do I miss any benefits provided by Caffeine?
   
   
   
   


-- 
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