sahlberg / libnfs

NFS client library
Other
511 stars 200 forks source link

Consistently use higher `maxcount` for READDIRPLUS #396

Closed rovo89 closed 1 year ago

rovo89 commented 1 year ago

The explanation is the same as in 4a58e6145568fd21ef956ae6ac3d3c37ef209a57, i.e. the server would gather 8192 bytes (dircount) of directory entries, fetch attributes for them and try to fit the extended entries into a 8192 bytes (maxcount) response. Obviously the attributes need quite some additional space, so only a small subset of the information can actually be sent back to the client, the rest is wasted.

Let's use the typical 8 to 1 ratio to allow more space for the response.

Benchmark with nfs-ls for different directories on a NetApp NFSv3 share:

    Files   Before    After
 -----------------------------------
    6,650   0.28s --> 0.14s  (-50%)
  163,128    4.8s -->  1.7s  (-65%)
  330,602    9.6s -->  3.2s  (-67%)

A significant improvement from ~34k to ~100k entries/s. Further increasing dircount/maxcount didn't lead to higher performance.

rovo89 commented 1 year ago

@sahlberg Any timeline for merging this? The performance numbers speak for themselves and https://github.com/sahlberg/libnfs/commit/4a58e6145568fd21ef956ae6ac3d3c37ef209a57 had the same reasoning 8 years ago. Would be great if I could refer to the latest master or even a new version instead of keeping an additional patch around. Thanks!

sahlberg commented 1 year ago

I will not up this limit by default. We/you can add a new api with a new function to push this limit higher but the default for nfv3 should remain 8kb for compatibility reasons.

sahlberg commented 1 year ago

Please add a new function nfs_set_readdir_max_buffer_size(...) to set it higher. That way we are still compatible with very old servers but also allow applications that only talk to new servers to override and set higher limits.

rovo89 commented 1 year ago

Closing this in favor of #404.