neoXfire commented on PR #771: URL: https://github.com/apache/httpcomponents-client/pull/771#issuecomment-3665788069
> > > Hello @ok2c, > > > Thank you for the quick feedback. > > > Please note one of the new pool implementation in core5 (`RouteSegmentedConnPool`) do not ship this `ConnPoolListener` interface. > > > I was wondering if I should also update the core so that pool event subscription is consistent between all implementations ? > > > > > > @neoXfire Yes, please do so. Could you please also add a TODO comment to `OFFLOCK` policy about missing `ConnPoolListener` support in both connection managers? > > I’m not convinced OFFLOCK needs ConnPoolListener at all. RouteSegmentedConnPool is intentionally lean / low-level: adding listener callbacks means extra branches and memory traffic on the hot lease/release paths, which is exactly what OFFLOCK is trying to minimize. I think with an appropriate implentation (notification callbacks stored in a `ConnPoolListener<T>` final field), in a sufficiently hot method and for the default case with no callbacks set up (connPoolListener == nul), the JIT can optimize this down to the same as if that if the conditional block for the callback event firing was never added. If there is no intent to drop STRICT and LAX support in the future to replace it by this new lock-free implementation OFFLOCK only, it is fine and I can probably stick to the two legacy implementations only for my change. However, if it is actually the plan, I think you should try to find the right balance between raw performance and thrid-party plugability (observability, instrumentation...). That last aspect is for a lot of people as important as the first one. Regards, -- 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]
