Skip to content

Commit

Permalink
Lots of changes to support latest WG21 SG14 status code, as the erase…
Browse files Browse the repository at this point in the history
…d code is now move-only, which in turn requires the async i/o completion handlers to all use rvalue refs instead of lvalue refs. This helped find and fix a fair few inefficient corner cases along the way, which is great.
  • Loading branch information
ned14 committed Oct 17, 2018
1 parent bc1a0df commit fd12a60
Show file tree
Hide file tree
Showing 26 changed files with 85 additions and 73 deletions.
6 changes: 3 additions & 3 deletions example/use_cases.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -101,9 +101,9 @@ void read_entire_file2()
auto ret = llfio::async_read( //
fh, // handle to read from
{ { scatter_req }, valid_extents[n].first }, // The scatter request buffers + offset
[]( // The completion handler
llfio::async_file_handle *, // The parent handle
llfio::async_file_handle::io_result<llfio::async_file_handle::buffers_type> & // Result of the i/o
[]( // The completion handler
llfio::async_file_handle *, // The parent handle
llfio::async_file_handle::io_result<llfio::async_file_handle::buffers_type> && // Result of the i/o
) { /* do nothing */ }
// default deadline is infinite
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -205,7 +205,7 @@ namespace algorithm
{
if(lockresult.error() != errc::timed_out)
{
return lockresult.error();
return std::move(lockresult).error();
}
// Somebody else is also using this file
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,7 @@ namespace algorithm
const auto &ec = ret.error();
if(ec != errc::resource_unavailable_try_again && ec != errc::file_exists)
{
return ret.error();
return std::move(ret).error();
}
// Collided with another locker
was_contended = n;
Expand Down
4 changes: 2 additions & 2 deletions include/llfio/v2.0/algorithm/shared_fs_mutex/memory_map.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -223,13 +223,13 @@ namespace algorithm
{
if(lockinuse.error() != errc::timed_out)
{
return lockinuse.error();
return std::move(lockinuse).error();
}
// Somebody else is also using this file, so try to read the hash index file I ought to use
lockinuse = ret.lock(_lockinuseoffset, 1, false); // inuse shared access, blocking
if(!lockinuse)
{
return lockinuse.error();
return std::move(lockinuse).error();
}
byte buffer[65536];
memset(buffer, 0, sizeof(buffer));
Expand Down
21 changes: 11 additions & 10 deletions include/llfio/v2.0/async_file_handle.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -225,7 +225,7 @@ class LLFIO_DECL async_file_handle : public file_handle
ret._service = &service;
return std::move(ret);
}
LLFIO_HEADERS_ONLY_VIRTUAL_SPEC result<file_handle> clone(mode mode_ = mode::unchanged, caching caching_ = caching::unchanged, deadline d = std::chrono::seconds(30)) const noexcept override
LLFIO_HEADERS_ONLY_VIRTUAL_SPEC result<file_handle> clone(mode mode_ = mode::unchanged, caching caching_ = caching::unchanged, deadline d = std::chrono::seconds(30)) const noexcept
{
OUTCOME_TRY(v, file_handle::clone(mode_, caching_, d));
async_file_handle ret(std::move(v));
Expand Down Expand Up @@ -265,6 +265,7 @@ class LLFIO_DECL async_file_handle : public file_handle
: read(buffers_type())
{
}
~result_storage() { /* needed as io_result is move-only when configured with status code */}
} result;
constexpr _erased_io_state_type(async_file_handle *_parent, operation_t _operation, bool _must_deallocate_self, size_t _items)
: parent(_parent)
Expand Down Expand Up @@ -376,7 +377,7 @@ class LLFIO_DECL async_file_handle : public file_handle
erased completion handler of unknown type and state per buffers input.
*/
LLFIO_MAKE_FREE_FUNCTION
template <class CompletionRoutine> //
template <class CompletionRoutine> //
LLFIO_REQUIRES(detail::is_invocable_r<void, CompletionRoutine, async_file_handle *, io_result<const_buffers_type> &>::value) //
result<io_state_ptr> async_barrier(io_request<const_buffers_type> reqs, CompletionRoutine &&completion, bool wait_for_device = false, bool and_metadata = false, span<char> mem = {}) noexcept
{
Expand All @@ -394,7 +395,7 @@ class LLFIO_DECL async_file_handle : public file_handle
auto *dest = reinterpret_cast<void *>(_dest);
new(dest) completion_handler(std::move(*this));
}
void operator()(_erased_io_state_type *state) final { completion(state->parent, state->result.write); }
void operator()(_erased_io_state_type *state) final { completion(state->parent, std::move(state->result.write)); }
void *address() noexcept final { return &completion; }
} ch{std::forward<CompletionRoutine>(completion)};
operation_t operation = operation_t::fsync_sync;
Expand Down Expand Up @@ -423,15 +424,15 @@ class LLFIO_DECL async_file_handle : public file_handle
\return Either an io_state_ptr to the i/o in progress, or an error code.
\param reqs A scatter-gather and offset request.
\param completion A callable to call upon i/o completion. Spec is `void(async_file_handle *, io_result<buffers_type> &)`.
\param completion A callable to call upon i/o completion. Spec is `void(async_file_handle *, io_result<buffers_type> &&)`.
Note that buffers returned may not be buffers input, see documentation for `read()`.
\param mem Optional span of memory to use to avoid using `calloc()`. Note span MUST be all bits zero on entry.
\errors As for `read()`, plus `ENOMEM`.
\mallocs If mem is not set, one calloc, one free. The allocation is unavoidable due to the need to store a type
erased completion handler of unknown type and state per buffers input.
*/
LLFIO_MAKE_FREE_FUNCTION
template <class CompletionRoutine> //
template <class CompletionRoutine> //
LLFIO_REQUIRES(detail::is_invocable_r<void, CompletionRoutine, async_file_handle *, io_result<buffers_type> &>::value) //
result<io_state_ptr> async_read(io_request<buffers_type> reqs, CompletionRoutine &&completion, span<char> mem = {}) noexcept
{
Expand All @@ -449,7 +450,7 @@ class LLFIO_DECL async_file_handle : public file_handle
auto *dest = reinterpret_cast<void *>(_dest);
new(dest) completion_handler(std::move(*this));
}
void operator()(_erased_io_state_type *state) final { completion(state->parent, state->result.read); }
void operator()(_erased_io_state_type *state) final { completion(state->parent, std::move(state->result.read)); }
void *address() noexcept final { return &completion; }
} ch{std::forward<CompletionRoutine>(completion)};
return _begin_io(mem, operation_t::read, io_request<const_buffers_type>({reinterpret_cast<const_buffer_type *>(reqs.buffers.data()), reqs.buffers.size()}, reqs.offset), std::move(ch));
Expand All @@ -466,15 +467,15 @@ class LLFIO_DECL async_file_handle : public file_handle
\return Either an io_state_ptr to the i/o in progress, or an error code.
\param reqs A scatter-gather and offset request.
\param completion A callable to call upon i/o completion. Spec is `void(async_file_handle *, io_result<const_buffers_type> &)`.
\param completion A callable to call upon i/o completion. Spec is `void(async_file_handle *, io_result<const_buffers_type> &&)`.
Note that buffers returned may not be buffers input, see documentation for `write()`.
\param mem Optional span of memory to use to avoid using `calloc()`. Note span MUST be all bits zero on entry.
\errors As for `write()`, plus `ENOMEM`.
\mallocs If mem in not set, one calloc, one free. The allocation is unavoidable due to the need to store a type
erased completion handler of unknown type and state per buffers input.
*/
LLFIO_MAKE_FREE_FUNCTION
template <class CompletionRoutine> //
template <class CompletionRoutine> //
LLFIO_REQUIRES(detail::is_invocable_r<void, CompletionRoutine, async_file_handle *, io_result<const_buffers_type> &>::value) //
result<io_state_ptr> async_write(io_request<const_buffers_type> reqs, CompletionRoutine &&completion, span<char> mem = {}) noexcept
{
Expand All @@ -492,7 +493,7 @@ class LLFIO_DECL async_file_handle : public file_handle
auto *dest = reinterpret_cast<void *>(_dest);
new(dest) completion_handler(std::move(*this));
}
void operator()(_erased_io_state_type *state) final { completion(state->parent, state->result.write); }
void operator()(_erased_io_state_type *state) final { completion(state->parent, std::move(state->result.write)); }
void *address() noexcept final { return &completion; }
} ch{std::forward<CompletionRoutine>(completion)};
return _begin_io(mem, operation_t::write, reqs, std::move(ch));
Expand All @@ -512,7 +513,7 @@ class LLFIO_DECL async_file_handle : public file_handle
optional<io_result<BuffersType>> _result;

// Called on completion of the i/o
void operator()(async_file_handle * /*unused*/, io_result<BuffersType> &result)
void operator()(async_file_handle * /*unused*/, io_result<BuffersType> &&result)
{
// store the result and resume the coroutine
_result = std::move(result);
Expand Down
18 changes: 9 additions & 9 deletions include/llfio/v2.0/detail/impl/posix/async_file_handle.ipp
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ async_file_handle::io_result<async_file_handle::const_buffers_type> async_file_h
{
LLFIO_LOG_FUNCTION_CALL(this);
optional<io_result<const_buffers_type>> ret;
OUTCOME_TRY(io_state, async_barrier(reqs, [&ret](async_file_handle *, io_result<const_buffers_type> &result) { ret = result; }, wait_for_device, and_metadata));
OUTCOME_TRY(io_state, async_barrier(reqs, [&ret](async_file_handle *, io_result<const_buffers_type> &&result) { ret = std::move(result); }, wait_for_device, and_metadata));
(void) io_state;

// While i/o is not done pump i/o completion
Expand All @@ -46,7 +46,7 @@ async_file_handle::io_result<async_file_handle::const_buffers_type> async_file_h
// If i/o service pump failed or timed out, cancel outstanding i/o and return
if(!t)
{
return t.error();
return std::move(t).error();
}
#ifndef NDEBUG
if(!ret && t && !t.value())
Expand All @@ -56,7 +56,7 @@ async_file_handle::io_result<async_file_handle::const_buffers_type> async_file_h
}
#endif
}
return *ret;
return std::move(*ret);
}

template <class BuffersType, class IORoutine> result<async_file_handle::io_state_ptr> async_file_handle::_begin_io(span<char> mem, async_file_handle::operation_t operation, async_file_handle::io_request<BuffersType> reqs, async_file_handle::_erased_completion_handler &&completion, IORoutine && /*unused*/) noexcept
Expand Down Expand Up @@ -328,7 +328,7 @@ async_file_handle::io_result<async_file_handle::buffers_type> async_file_handle:
{
LLFIO_LOG_FUNCTION_CALL(this);
optional<io_result<buffers_type>> ret;
OUTCOME_TRY(io_state, async_read(reqs, [&ret](async_file_handle *, io_result<buffers_type> &result) { ret = result; }));
OUTCOME_TRY(io_state, async_read(reqs, [&ret](async_file_handle *, io_result<buffers_type> &&result) { ret = std::move(result); }));
(void) io_state;

// While i/o is not done pump i/o completion
Expand All @@ -338,7 +338,7 @@ async_file_handle::io_result<async_file_handle::buffers_type> async_file_handle:
// If i/o service pump failed or timed out, cancel outstanding i/o and return
if(!t)
{
return t.error();
return std::move(t).error();
}
#ifndef NDEBUG
if(!ret && t && !t.value())
Expand All @@ -348,14 +348,14 @@ async_file_handle::io_result<async_file_handle::buffers_type> async_file_handle:
}
#endif
}
return *ret;
return std::move(*ret);
}

async_file_handle::io_result<async_file_handle::const_buffers_type> async_file_handle::write(async_file_handle::io_request<async_file_handle::const_buffers_type> reqs, deadline d) noexcept
{
LLFIO_LOG_FUNCTION_CALL(this);
optional<io_result<const_buffers_type>> ret;
OUTCOME_TRY(io_state, async_write(reqs, [&ret](async_file_handle *, io_result<const_buffers_type> &result) { ret = result; }));
OUTCOME_TRY(io_state, async_write(reqs, [&ret](async_file_handle *, io_result<const_buffers_type> &&result) { ret = std::move(result); }));
(void) io_state;

// While i/o is not done pump i/o completion
Expand All @@ -365,7 +365,7 @@ async_file_handle::io_result<async_file_handle::const_buffers_type> async_file_h
// If i/o service pump failed or timed out, cancel outstanding i/o and return
if(!t)
{
return t.error();
return std::move(t).error();
}
#ifndef NDEBUG
if(!ret && t && !t.value())
Expand All @@ -375,7 +375,7 @@ async_file_handle::io_result<async_file_handle::const_buffers_type> async_file_h
}
#endif
}
return *ret;
return std::move(*ret);
}

LLFIO_V2_NAMESPACE_END
2 changes: 1 addition & 1 deletion include/llfio/v2.0/detail/impl/posix/directory_handle.ipp
Original file line number Diff line number Diff line change
Expand Up @@ -172,7 +172,7 @@ result<directory_handle> directory_handle::clone(mode mode_, caching caching_, d
{
if(fh.error() != errc::no_such_file_or_directory)
{
return fh.error();
return std::move(fh).error();
}
}
// Check timeout
Expand Down
2 changes: 1 addition & 1 deletion include/llfio/v2.0/detail/impl/posix/file_handle.ipp
Original file line number Diff line number Diff line change
Expand Up @@ -308,7 +308,7 @@ result<file_handle> file_handle::clone(mode mode_, caching caching_, deadline d)
{
if(fh.error() != errc::no_such_file_or_directory)
{
return fh.error();
return std::move(fh).error();
}
}
// Check timeout
Expand Down
8 changes: 5 additions & 3 deletions include/llfio/v2.0/detail/impl/posix/symlink_handle.ipp
Original file line number Diff line number Diff line change
Expand Up @@ -185,7 +185,7 @@ result<symlink_handle> symlink_handle::clone(mode mode_, deadline d) const noexc
{
if(fh.error() != errc::no_such_file_or_directory)
{
return fh.error();
return std::move(fh).error();
}
}
// Check timeout
Expand Down Expand Up @@ -357,7 +357,9 @@ LLFIO_HEADERS_ONLY_MEMFUNC_SPEC result<symlink_handle> symlink_handle::symlink(c
if(!r)
{
if(_creation == creation::only_if_not_exist || r.error() != errc::file_exists)
return r.error();
{
return std::move(r).error();
}
}
break;
}
Expand Down Expand Up @@ -456,7 +458,7 @@ result<symlink_handle::const_buffers_type> symlink_handle::write(symlink_handle:
// If reopen failed, report that now
if(!newthis)
{
return newthis.error();
return std::move(newthis).error();
}
// Swap myself with the new this
swap(newthis.value());
Expand Down
12 changes: 6 additions & 6 deletions include/llfio/v2.0/detail/impl/windows/async_file_handle.ipp
Original file line number Diff line number Diff line change
Expand Up @@ -223,7 +223,7 @@ async_file_handle::io_result<async_file_handle::buffers_type> async_file_handle:
{
LLFIO_LOG_FUNCTION_CALL(this);
optional<io_result<buffers_type>> ret;
OUTCOME_TRY(io_state, async_read(reqs, [&ret](async_file_handle *, io_result<buffers_type> &result) { ret = result; }));
OUTCOME_TRY(io_state, async_read(reqs, [&ret](async_file_handle *, io_result<buffers_type> &&result) { ret = std::move(result); }));
(void) io_state; // holds i/o open until it completes

// While i/o is not done pump i/o completion
Expand All @@ -233,7 +233,7 @@ async_file_handle::io_result<async_file_handle::buffers_type> async_file_handle:
// If i/o service pump failed or timed out, cancel outstanding i/o and return
if(!t)
{
return t.error();
return std::move(t).error();
}
#ifndef NDEBUG
if(!ret && t && !t.value())
Expand All @@ -243,14 +243,14 @@ async_file_handle::io_result<async_file_handle::buffers_type> async_file_handle:
}
#endif
}
return *ret;
return std::move(*ret);
}

async_file_handle::io_result<async_file_handle::const_buffers_type> async_file_handle::write(async_file_handle::io_request<async_file_handle::const_buffers_type> reqs, deadline d) noexcept
{
LLFIO_LOG_FUNCTION_CALL(this);
optional<io_result<const_buffers_type>> ret;
OUTCOME_TRY(io_state, async_write(reqs, [&ret](async_file_handle *, io_result<const_buffers_type> &result) { ret = result; }));
OUTCOME_TRY(io_state, async_write(reqs, [&ret](async_file_handle *, io_result<const_buffers_type> &&result) { ret = std::move(result); }));
(void) io_state; // holds i/o open until it completes

// While i/o is not done pump i/o completion
Expand All @@ -260,7 +260,7 @@ async_file_handle::io_result<async_file_handle::const_buffers_type> async_file_h
// If i/o service pump failed or timed out, cancel outstanding i/o and return
if(!t)
{
return t.error();
return std::move(t).error();
}
#ifndef NDEBUG
if(!ret && t && !t.value())
Expand All @@ -270,7 +270,7 @@ async_file_handle::io_result<async_file_handle::const_buffers_type> async_file_h
}
#endif
}
return *ret;
return std::move(*ret);
}


Expand Down
2 changes: 1 addition & 1 deletion include/llfio/v2.0/detail/impl/windows/fs_handle.ipp
Original file line number Diff line number Diff line change
Expand Up @@ -251,7 +251,7 @@ result<void> fs_handle::unlink(deadline d) noexcept
}
else
{
return out.error();
return std::move(out).error();
}
}
}
Expand Down
2 changes: 1 addition & 1 deletion include/llfio/v2.0/directory_handle.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -261,7 +261,7 @@ class LLFIO_DECL directory_handle : public path_handle, public fs_handle
// File may have already been deleted, if so ignore
if(ret.error() != errc::no_such_file_or_directory)
{
return ret.error();
return std::move(ret).error();
}
}
}
Expand Down
6 changes: 3 additions & 3 deletions include/llfio/v2.0/file_handle.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -204,7 +204,7 @@ class LLFIO_DECL file_handle : public io_handle, public fs_handle
// File may have already been deleted, if so ignore
if(ret.error() != errc::no_such_file_or_directory)
{
return ret.error();
return std::move(ret).error();
}
}
}
Expand Down Expand Up @@ -232,7 +232,7 @@ class LLFIO_DECL file_handle : public io_handle, public fs_handle
\mallocs On POSIX if changing the mode, we must loop calling `current_path()` and
trying to open the path returned. Thus many allocations may occur.
*/
LLFIO_HEADERS_ONLY_VIRTUAL_SPEC result<file_handle> clone(mode mode_ = mode::unchanged, caching caching_ = caching::unchanged, deadline d = std::chrono::seconds(30)) const noexcept;
result<file_handle> clone(mode mode_ = mode::unchanged, caching caching_ = caching::unchanged, deadline d = std::chrono::seconds(30)) const noexcept;

//! The i/o service this handle is attached to, if any
io_service *service() const noexcept { return _service; }
Expand All @@ -250,7 +250,7 @@ class LLFIO_DECL file_handle : public io_handle, public fs_handle
{
return ret.bytes_transferred();
}
return ret.error();
return std::move(ret).error();
}

/*! Return the current maximum permitted extent of the file.
Expand Down
Loading

0 comments on commit fd12a60

Please sign in to comment.