adriangb commented on code in PR #16433:
URL: https://github.com/apache/datafusion/pull/16433#discussion_r2282817078
##########
datafusion/physical-expr/src/expressions/dynamic_filters.rs:
##########
@@ -137,7 +139,7 @@ impl DynamicFilterPhysicalExpr {
Self {
children,
remapped_children: None, // Initially no remapped children
- inner: Arc::new(RwLock::new(Inner::new(inner))),
+ inner: Arc::new(ArcSwap::from_pointee(Inner::new(inner))),
Review Comment:
Well I stumbled upon it because we originally had some performance
regressions and I suspected they might be due to hitting the `Arc<RwLock<>>`s a
lot heavier. It seems that `ArcSwap` is the solution to that but I haven't
benchmarked to confirm the performance impact / if it's that or other
refactors. From a micro benchmark sitauation it makes sense. I can make a
branch / another PR without ArcSwap and we can see if there's a measurable
change at the macro level, but I expect it may be hard to measure.
That said it's a well maintained dep from a reputable maintainer, has a lot
of usage and has no dependencies of its own.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
