Commits on Jan 24, 2018

1. fs: partition readFile against threadpool exhaustion …

   Problem:
   
   Node implements fs.readFile as:
   - a call to stat, then
   - a C++ -> libuv request to read the entire file using the stat size
   
   Why is this bad?
   The effect is to place on the libuv threadpool a potentially-large
   read request, occupying the libuv thread until it completes.
   While readFile certainly requires buffering the entire file contents,
   it can partition the read into smaller buffers
   (as is done on other read paths)
   along the way to avoid threadpool exhaustion.
   
   If the file is relatively large or stored on a slow medium, reading
   the entire file in one shot seems particularly harmful,
   and presents a possible DoS vector.
   
   Solution:
   
   Partition the read into multiple smaller requests.
   
   Considerations:
   
   1. Correctness
   
   I don't think partitioning the read like this raises
   any additional risk of read-write races on the FS.
   If the application is concurrently readFile'ing and modifying the file,
   it will already see funny behavior. Though libuv uses preadv where
   available, this doesn't guarantee read atomicity in the presence of
   concurrent writes.
   
   2. Performance
   
   Downside: Partitioning means that a single large readFile will
     require into many "out and back" requests to libuv,
     introducing overhead.
   Upside: In between each "out and back", other work pending on the
     threadpool can take a turn.
   
   In short, although partitioning will slow down a large request,
   it will lead to better throughput if the threadpool is handling
   more than one type of request.
   
   Test:
   
   I added test/parallel/test-fs-readfile.js.
   
   Benchmark:
   
   I introduced benchmark/fs/readfile-partitioned.js to characterize
   the performance tradeoffs.
   See PR for details.
   
   Related:
   
   It might be that writeFile has similar behavior.
   The writeFile path is a bit more complex and I didn't
   investigate carefully.
   
   Fixes: nodejs#17047

   

   davisjam committed Jan 24, 2018
   Configuration menu

   • View commit details
   • Copy full SHA for 30ac024
   • Browse repository at this point

   Copy the full SHA

   30ac024 View commit details

   Browse the repository at this point in the history