gribozavr added inline comments.
================ Comment at: clang/lib/DirectoryWatcher/linux/DirectoryWatcher-linux.cpp:136 + + const int PollResult = poll(&PollReq, 1, TimeoutMs); + // There are inotify events waiting to be read! ---------------- jkorous wrote: > gribozavr wrote: > > What is the role of the timeout and why does it need to be so small? > The whole idea is that we can't block on `read()` if we ever want to stop > watching the directory, release resources (file descriptors, threads) and > correctly destruct the DirectoryWatcher instance either > - because of a bug in some other thread in the implementation > - or asynchronous client action (e. g. destructor being called) in main > application thread > > The timeout adds latency in those scenarios. Waking up 1000 times a second is not great -- it will lead to battery drain on laptops etc. Please see https://stackoverflow.com/questions/8593004/waiting-on-a-condition-pthread-cond-wait-and-a-socket-change-select-simultan for non-busy-wait options. ================ Comment at: clang/lib/DirectoryWatcher/linux/DirectoryWatcher-linux.cpp:111 + + std::unique_ptr<llvm::AlignedCharArray<__alignof__(struct inotify_event), + EventBufferLength>> ---------------- "alignof()" expression is standard C++ since C++11. (No need for underscores.) ================ Comment at: clang/lib/DirectoryWatcher/linux/DirectoryWatcher-linux.cpp:114 + ManagedBuffer = llvm::make_unique<llvm::AlignedCharArray< + __alignof__(struct inotify_event), EventBufferLength>>(); + char *const Buf = ManagedBuffer->buffer; ---------------- use 'auto' to store the return value of make_unique? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D58418/new/ https://reviews.llvm.org/D58418 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
