[GitHub] [arrow-datafusion] ozankabak commented on pull request #4079: Replace RwLock and Mutex by using DashMap

[GitBox <gi...@apache.org>]

ozankabak commented on PR #4079:
URL: https://github.com/apache/arrow-datafusion/pull/4079#issuecomment-1301886965

   My two cents: I think it is more about encapsulating certain low-level operations regarding the data structure within the data structure itself, vs. exposing them in higher-level code. I think this is probably what @xudong963 means by cleaner, and I agree.
   
   I also agree with @tustvold that typical workloads will not result in contention, without which performance improvements will not manifest.
   
   If this change would merge, the benefits would be:
   1. Code with better separation of concern (although the benefit is not as much as the Ballista change)
   2. Potential performance gain in the unrealistic-looking contention scenarios
   3. Being more in-line with Ballista codebase
   The main disadvantage that I can see:
   1. Introducing a dependency to Datafusion (albeit it is fairly commonly-used one)
   
   I don't think the argument either way is super strong, but it seems to me that pros somewhat outweigh the cons (unless I am missing something).


-- 
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: github-unsubscribe@arrow.apache.org

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