tomaswolf edited a comment on pull request #207: URL: https://github.com/apache/mina-sshd/pull/207#issuecomment-955198554
> The code is certainly more complex to follow... It seems OK, but I may have missed something. Perhaps you should ask Guillaume to take a look at it as well - maybe he can locate something we both missed. In view of this I definitely recommend we make this a configurable behavior (default=enabled) Well, I think you know how I feel about this... Anyway: added such a config. But IMO it only adds unnecessary complication to the code, and users should never set it to false anyway. It really hurts performance. Try the directory listing test with it set to true or false: - true: takes some 70ms per run on my machine, makes 3 LSTAT calls - false: takes >2s per run on my machine, makes 10040 LSTAT calls The caching of attributes is truly _safe_: the unconditionally cached attributes are used _only_ in directory iteration; just like the cached attributes from `sun.nio.fs.BasicFileAttributesHolder` are used exclusively in `FileTreeWalker` in the Java library. And `withAttributeCaching` is used only for short code blocks that are _known_ to only make repeated calls to `readAttributes`, and they clear the cache at the end. My proposition instead of a pointless config: either we believe this implementation is correct and then check it in without config, or we believe it isn't correct and then we abandon this idea altogether. And if we can't decide what we believe, then let's also abandon this. -- 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: dev-unsubscr...@mina.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@mina.apache.org For additional commands, e-mail: dev-h...@mina.apache.org
