Skip to content

Commit

Permalink
Unify writev and no-writev code paths in lsp_pipe_writer
Browse files Browse the repository at this point in the history
lsp_pipe_writer has two code paths:

* writev: convert the message byte_buffer into a byte_buffer_iovec, and
  use the writev syscall to write data to the pipe
* no-writev: convert the message byte_buffer into a string8, and use the
  write/WriteFile syscall to write data to the pipe

Having two code paths complicates future changes, such as scheduling
WriteFile on a separate thread [1]. Settle on one code path as much as
possible:

* writev: convert the message byte_buffer into a byte_buffer_iovec, and
  use the writev syscall to write data to the pipe
* no-writev: convert the message byte_buffer into a byte_buffer_iovec
  (same code as if writev is supported), and use the write/WriteFile
  syscall to write data to the pipe

This commit possibly harms performance by calling WriteFile more (with
smaller buffers) on Windows. I didn't attempt to measure this
performance difference.

[1]: #229
  • Loading branch information
strager committed Jun 13, 2021
1 parent 4d71651 commit 9116a2d
Show file tree
Hide file tree
Showing 2 changed files with 15 additions and 34 deletions.
46 changes: 15 additions & 31 deletions src/lsp-pipe-writer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -36,52 +36,36 @@ char8* make_header(std::size_t message_size, char8* out) {
lsp_pipe_writer::lsp_pipe_writer(platform_file_ref pipe) : pipe_(pipe) {}

void lsp_pipe_writer::send_message(byte_buffer&& message) {
#if QLJS_HAVE_WRITEV
std::array<char8, max_header_size> header;
char8* header_end = make_header(message.size(), header.data());
message.prepend_copy(string8_view(
header.data(), narrow_cast<std::size_t>(header_end - header.data())));
this->write(std::move(message).to_iovec());
#else
// TODO(strager): Avoid std::basic_string's bloat. (SSO will never kick in.)
string8 data;
std::size_t message_size = message.size();
data.resize(message_size + max_header_size);
char8* out = data.data();

out = make_header(message_size, out);
message.copy_to(out);
out += message_size;

this->write(
string8_view(data.data(), narrow_cast<std::size_t>(out - data.data())));
#endif
}

void lsp_pipe_writer::write(string8_view data) {
while (!data.empty()) {
std::optional<int> bytes_written =
this->pipe_.write(data.data(), narrow_cast<int>(data.size()));
if (!bytes_written.has_value()) {
QLJS_UNIMPLEMENTED();
}
data = data.substr(narrow_cast<std::size_t>(*bytes_written));
}
}

#if QLJS_HAVE_WRITEV
void lsp_pipe_writer::write(byte_buffer_iovec&& data) {
while (data.iovec_count() != 0) {
::ssize_t bytes_written =
#if QLJS_HAVE_WRITEV
::ssize_t raw_bytes_written =
::writev(this->pipe_.get(), data.iovec(), data.iovec_count());
if (bytes_written < 0) {
if (raw_bytes_written < 0) {
QLJS_UNIMPLEMENTED();
}
std::size_t bytes_written = narrow_cast<std::size_t>(raw_bytes_written);
#else
const byte_buffer_chunk& chunk = data.iovec()[0];
QLJS_ASSERT(chunk.size != 0); // Writing can hang if given size 0.
std::optional<int> raw_bytes_written =
this->pipe_.write(chunk.data, narrow_cast<int>(chunk.size));
if (!raw_bytes_written.has_value()) {
QLJS_UNIMPLEMENTED();
}
std::size_t bytes_written = narrow_cast<std::size_t>(*raw_bytes_written);
#endif
QLJS_ASSERT(bytes_written != 0);
data.remove_front(narrow_cast<std::size_t>(bytes_written));
data.remove_front(bytes_written);
}
}
#endif
}

// quick-lint-js finds bugs in JavaScript programs.
Expand Down
3 changes: 0 additions & 3 deletions src/quick-lint-js/lsp-pipe-writer.h
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,7 @@ class lsp_pipe_writer {
void send_message(byte_buffer&&);

private:
void write(string8_view);
#if QLJS_HAVE_WRITEV
void write(byte_buffer_iovec&&);
#endif

platform_file_ref pipe_;
};
Expand Down

0 comments on commit 9116a2d

Please sign in to comment.