iprithv commented on PR #16173: URL: https://github.com/apache/lucene/pull/16173#issuecomment-5197324290
thanks @msfroh, I checked the issue you linked..so issue with https://github.com/apache/lucene/pull/15469 (openChecksumInput) was that it calls back into openInput() on itself, so delegating it bypassed the filter's interception of openInput(). that's a correctness bug. but here copyFrom() is actually different, it's a self-contained operation. HardlinkCopyDirectoryWrapper.copyFrom() explicitly delegates to getDelegate().copyFrom(), not this.copyFrom(). without FilterDirectory delegating copyFrom(), wrapping HardlinkCopyDirectoryWrapper silently loses the hardlink optimization.. a real performance regression that's hard to debug. I believe general rule could be..delegate methods that are self-contained optimizations (like copyFrom()), but don't delegate methods that call back into overridable methods (like openChecksumInput()). I can add a test that explicitly verifies custom copyFrom() implementations are preserved through wrapping, if that would help :slightly_smiling_face: WDYT? -- 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]
