Closed akyrtzi closed 6 days ago
@swift-ci Please test LLVM
I put back the flush();
call in pwrite_impl()
, otherwise SupportTests/VirtualOutputBackendAdaptors/makeMirroringOutputBackend
unit test is failing.
@swift-ci Please test LLVM
I put back the
flush();
call inpwrite_impl()
, otherwiseSupportTests/VirtualOutputBackendAdaptors/makeMirroringOutputBackend
unit test is failing.
If it is only that test, it is fine to flush in the unit test before checking content.
If it is only that test, it is fine to flush in the unit test before checking content.
My worry is that the test failure is indication that the flush
call is necessary to support the behavioral expectations of pwrite
callers.
MirroringOutput::pwrite_impl()
is not called during compilation so I'm not so worried about it, we can revisit after input from @dexonsmith
The unbuffered behaviour was intentional, and it seemed important, but I can't remember why... it's possible it only mattered for some clients (and it's possible those clients were hypothetical).
Maybe this was for live display of command-line output? (Or stderr?)
Or maybe it was something more subtle, like ensuring there's only one buffer, rather than a chain of buffers... at some point I had to do funny things with buffers to fix a weird crash after compilation failed.
@dexonsmith could you comment on this override of MirroringOutput
:
void pwrite_impl(const char *Ptr, size_t Size, uint64_t Offset) override {
this->flush();
F1->getOS().pwrite(Ptr, Size, Offset);
F2->getOS().pwrite(Ptr, Size, Offset);
}
pwrite_impl()
is not called during compilation but I was worried the flush()
call may cause some performance issue if it does get used. I did notice that the SupportTests/VirtualOutputBackendAdaptors/makeMirroringOutputBackend
unit test depends on the presence of the flush()
call.
Should we not be worried about performance issues related to calling flush()
for each pwrite
call?
MirroringOutputBackend
was forcing theraw_ostream
s to be unbuffered, causing significant slowdown due to I/O. When enabling Clang caching for building Clang or WebKit, the "from scratch" (all cache misses) build was slower than the regular non-caching build. For building debug Clang the overhead was ~22%, and for debug build of WebKit, it was ~15%.The overhead went away after removing the
SetUnbuffered()
calls.rdar://130514092