Skip to content

Commit 87eeacb

Browse files
devshgraphicsprogrammingdevshgraphicsprogramming
committed
figure out how we're gonna construct the results over the future's storage
1 parent b535556 commit 87eeacb

File tree

5 files changed

+150
-144
lines changed

5 files changed

+150
-144
lines changed

‎include/nbl/asset/IAssetManager.h

Lines changed: 7 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -330,14 +330,12 @@ class NBL_API2 IAssetManager : public core::IReferenceCounted, public core::Quit
330330
filePath = _params.workingDirectory/filePath;
331331
_override->getLoadFilename(filePath, m_system.get(), ctx, _hierarchyLevel);
332332
}
333-
333+
334334
system::ISystem::future_t<core::smart_refctd_ptr<system::IFile>> future;
335335
m_system->createFile(future, filePath, system::IFile::ECF_READ);
336-
auto file = future.get();
337-
if (!file)
338-
return SAssetBundle(0);
339-
340-
return getAssetInHierarchy_impl<RestoreWholeBundle>(file.get(), filePath.string(), ctx.params, _hierarchyLevel, _override);
336+
if (auto file=future.acquire())
337+
return getAssetInHierarchy_impl<RestoreWholeBundle>(file->get(), filePath.string(), ctx.params, _hierarchyLevel, _override);
338+
return SAssetBundle(0);
341339
}
342340

343341
//TODO change name
@@ -653,14 +651,9 @@ class NBL_API2 IAssetManager : public core::IReferenceCounted, public core::Quit
653651

654652
system::ISystem::future_t<core::smart_refctd_ptr<system::IFile>> future;
655653
m_system->createFile(future, (_params.workingDirectory.generic_string()+_filename).c_str(), system::IFile::ECF_WRITE);
656-
core::smart_refctd_ptr<system::IFile>* file;
657-
if (future.wait() && (file=future.acquire()))
658-
{
659-
bool res = writeAsset(file->get(), _params, _override);
660-
future.release();
661-
return res;
662-
}
663-
return false;
654+
if (auto file=future.acquire())
655+
return writeAsset(file->get(), _params, _override);
656+
return false;
664657
}
665658
bool writeAsset(system::IFile* _file, const IAssetWriter::SAssetWriteParams& _params, IAssetWriter::IAssetWriterOverride* _override)
666659
{

‎include/nbl/system/IAsyncQueueDispatcher.h

Lines changed: 85 additions & 79 deletions
Original file line numberDiff line numberDiff line change
@@ -138,20 +138,18 @@ class IAsyncQueueDispatcherBase
138138

139139
public:
140140
template<typename T>
141-
class future_t : private core::StorageTrivializer<T>, protected future_base_t
141+
class future_t : private core::StorageTrivializer<T>, public future_base_t
142142
{
143143
using storage_t = core::StorageTrivializer<T>;
144-
inline void discard_common()
145-
{
146-
storage_t::destruct();
147-
state.exchangeNotify<true>(STATE::INITIAL,STATE::LOCKED);
148-
}
144+
145+
friend class storage_lock_t;
149146

150147
public:
151148
inline future_t() : future_base_t() {}
152-
inline ~future_t()
149+
virtual inline ~future_t()
153150
{
154-
discard();
151+
if (auto lock=acquire())
152+
lock.discard();
155153
}
156154

157155
//!
@@ -193,70 +191,93 @@ class IAsyncQueueDispatcherBase
193191
return retval;
194192
}
195193

196-
//! NOTE: Deliberately named `...acquire` instead of `...lock` to make them incompatible with `unique_lock`
197-
// and other RAII locks as the blocking aquire can fail and that needs to be handled.
198-
199-
//! ANY THREAD [except WORKER]: If we're READY transition to LOCKED
200-
[[nodiscard]] inline T* try_acquire()
201-
{
202-
auto expected = STATE::READY;
203-
if (state.tryTransition(STATE::LOCKED,expected))
204-
return storage_t::getStorage();
205-
return nullptr;
206-
}
207-
//! ANY THREAD [except WORKER]: Wait till we're either in READY and move us to LOCKED or bail on INITIAL
208-
// this accounts for being cancelled or consumed while waiting
209-
[[nodiscard]] inline T* acquire()
210-
{
211-
if (state.waitAbortableTransition(STATE::LOCKED,STATE::READY,STATE::INITIAL))
212-
return storage_t::getStorage();
213-
return nullptr;
214-
}
215-
//! ANY THREAD [except WORKER]: Release an acquired lock
216-
inline void release()
217-
{
218-
state.exchangeNotify<true>(STATE::READY,STATE::LOCKED);
219-
}
220-
221194
//! NOTE: You're in charge of ensuring future doesn't transition back to INITIAL (e.g. lock or use sanely!)
222195
inline const T* get() const
223196
{
224197
if (ready())
225198
return storage_t::getStorage();
226199
return nullptr;
227200
}
228-
inline T* get()
201+
202+
//! Utility to write less code, WILL ASSERT IF IT FAILS! So don't use on futures that might be cancelled or fail.
203+
inline T copy() const
229204
{
230-
if (future_base_t::state.query() != future_base_t::STATE::LOCKED)
231-
return nullptr;
232-
return storage_t::getStorage();
205+
const bool success = wait();
206+
assert(success);
207+
return *get();
233208
}
234209

235-
//! Can only be called once! If returns false means has been cancelled and nothing happened
236-
[[nodiscard]] inline bool move_into(T& dst)
210+
//! NOTE: Deliberately named `...acquire` instead of `...lock` to make them incompatible with `std::unique_lock`
211+
// and other RAII locks as the blocking aquire can fail and that needs to be handled.
212+
class storage_lock_t final
237213
{
238-
T* pSrc = acquire();
239-
if (!pSrc)
240-
return false;
241-
dst = std::move(*pSrc);
242-
discard_common();
243-
return true;
244-
}
214+
using state_enum = future_base_t::STATE;
215+
future_t<T>* m_future;
216+
217+
//! constructor, arg is nullptr if locked
218+
friend class future_t<T>;
219+
inline storage_lock_t(future_t<T>* _future) : m_future(_future)
220+
{
221+
assert(m_future->state.query()==state_enum::LOCKED);
222+
}
223+
//! as usual for "unique" things
224+
inline storage_lock_t(const storage_lock_t&) = delete;
225+
inline storage_lock_t& operator=(const storage_lock_t&) = delete;
226+
227+
public:
228+
inline ~storage_lock_t()
229+
{
230+
if (m_future)
231+
m_future->state.exchangeNotify<true>(state_enum::READY,state_enum::LOCKED);
232+
}
233+
234+
//!
235+
inline explicit operator bool()
236+
{
237+
return m_future;
238+
}
239+
inline bool operator!()
240+
{
241+
return !m_future;
242+
}
245243

246-
//! Utility to write less code, WILL ASSERT IF IT FAILS TO ACQUIRE! So don't use on futures that might be cancelled or fail.
247-
inline T copy_out()
244+
//!
245+
inline T* operator->() const
246+
{
247+
if (m_future)
248+
return m_future->getStorage();
249+
return nullptr;
250+
}
251+
template<typename U=T> requires (std::is_same_v<U,T> && !std::is_void_v<U>)
252+
inline U& operator*() const {return *operator->();}
253+
254+
//! Can only be called once!
255+
inline void discard()
256+
{
257+
assert(m_future);
258+
m_future->destruct();
259+
m_future->state.exchangeNotify<true>(state_enum::INITIAL,state_enum::LOCKED);
260+
}
261+
//! Can only be called once!
262+
template<typename U=T> requires (std::is_same_v<U,T> && !std::is_void_v<U>)
263+
inline void move_into(U& dst)
264+
{
265+
dst = std::move(operator*());
266+
discard();
267+
}
268+
};
269+
270+
//! ANY THREAD [except WORKER]: If we're READY transition to LOCKED
271+
inline storage_lock_t try_acquire()
248272
{
249-
T retval;
250-
const bool success = move_into(retval);
251-
assert(success);
252-
return retval;
273+
auto expected = STATE::READY;
274+
return storage_lock_t(state.tryTransition(STATE::LOCKED,expected) ? this:nullptr);
253275
}
254-
255-
//!
256-
virtual inline void discard()
276+
//! ANY THREAD [except WORKER]: Wait till we're either in READY and move us to LOCKED or bail on INITIAL
277+
// this accounts for being cancelled or consumed while waiting
278+
inline storage_lock_t acquire()
257279
{
258-
if (acquire())
259-
discard_common();
280+
return storage_lock_t(state.waitAbortableTransition(STATE::LOCKED,STATE::READY,STATE::INITIAL) ? this:nullptr);
260281
}
261282

262283
protected:
@@ -276,6 +297,12 @@ class IAsyncQueueDispatcherBase
276297
std::atomic<request_base_t*> request = nullptr;
277298

278299
public:
300+
inline ~cancellable_future_t()
301+
{
302+
// try to cancel
303+
cancel();
304+
}
305+
279306
//! ANY THREAD [except WORKER]: Cancel pending request if we can, returns whether we actually managed to cancel
280307
inline bool cancel()
281308
{
@@ -305,16 +332,6 @@ class IAsyncQueueDispatcherBase
305332
return false;
306333
}
307334

308-
inline void discard() override final
309-
{
310-
// try to cancel
311-
cancel();
312-
// sanity check
313-
assert(request==nullptr);
314-
// proceed with the usual
315-
base_t::discard();
316-
}
317-
318335
private:
319336
inline void associate_request(request_base_t* req) override final
320337
{
@@ -338,18 +355,7 @@ class IAsyncQueueDispatcherBase
338355

339356
protected:
340357
template<typename T>
341-
class future_constructor_t final
342-
{
343-
future_t<T>* pFuture;
344-
public:
345-
inline future_constructor_t(future_base_t* _future_base) : pFuture(static_cast<future_t<T>*>(_future_base)) {}
346-
347-
template<typename... Args>
348-
inline void operator()(Args&&... args)
349-
{
350-
pFuture->construct(std::forward<Args>(args)...);
351-
}
352-
};
358+
static inline core::StorageTrivializer<T>* future_storage_cast(future_base_t* _future_base) {return static_cast<future_t<T>*>(_future_base);}
353359
};
354360

355361
inline void IAsyncQueueDispatcherBase::request_base_t::finalize(future_base_t* fut)

‎include/nbl/system/ISystem.h

Lines changed: 20 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ class NBL_API2 ISystem : public core::IReferenceCounted
2828
struct future_t final : public impl::IAsyncQueueDispatcherBase::cancellable_future_t<T>
2929
{
3030
private:
31+
friend class ISystem;
3132
friend class IFutureManipulator;
3233

3334
using base_t = impl::IAsyncQueueDispatcherBase::cancellable_future_t<T>;
@@ -59,7 +60,7 @@ class NBL_API2 ISystem : public core::IReferenceCounted
5960
future_t<core::smart_refctd_ptr<IFile>> future;
6061
createFile(future,system::path(Path.value),core::bitflag(IFileBase::ECF_READ)|IFileBase::ECF_MAPPABLE);
6162
if (future.wait())
62-
return future.copy_out();
63+
return future.copy();
6364
return nullptr;
6465
#endif
6566
}
@@ -130,7 +131,7 @@ class NBL_API2 ISystem : public core::IReferenceCounted
130131
createFile(future, filename, core::bitflag<IFileBase::E_CREATE_FLAGS>(IFileBase::ECF_READ)|IFileBase::ECF_MAPPABLE);
131132

132133
if (future.wait())
133-
return openFileArchive(future.copy_out(),password);
134+
return openFileArchive(future.copy(),password);
134135
return nullptr;
135136
}
136137

@@ -218,27 +219,36 @@ class NBL_API2 ISystem : public core::IReferenceCounted
218219
} m_loaders;
219220
//
220221
core::CMultiObjectCache<system::path,core::smart_refctd_ptr<IFileArchive>> m_cachedArchiveFiles;
221-
222-
protected:
223-
class ICaller;
222+
224223
private:
225224
struct SRequestParams_NOOP
226225
{
226+
using retval_t = void;
227+
inline void operator()(core::StorageTrivializer<retval_t>* retval, ICaller* _caller) {}
227228
};
228229
struct SRequestParams_CREATE_FILE
229230
{
231+
using retval_t = core::smart_refctd_ptr<IFile>;
232+
void operator()(core::StorageTrivializer<retval_t>* retval, ICaller* _caller);
233+
230234
char filename[MAX_FILENAME_LENGTH] {};
231235
IFileBase::E_CREATE_FLAGS flags;
232236
};
233237
struct SRequestParams_READ
234238
{
239+
using retval_t = size_t;
240+
void operator()(core::StorageTrivializer<retval_t>* retval, ICaller* _caller);
241+
235242
ISystemFile* file;
236243
void* buffer;
237244
size_t offset;
238245
size_t size;
239246
};
240247
struct SRequestParams_WRITE
241248
{
249+
using retval_t = size_t;
250+
void operator()(core::StorageTrivializer<retval_t>* retval, ICaller* _caller);
251+
242252
ISystemFile* file;
243253
const void* buffer;
244254
size_t offset;
@@ -254,27 +264,20 @@ class NBL_API2 ISystem : public core::IReferenceCounted
254264
> params = SRequestParams_NOOP();
255265
};
256266
static inline constexpr uint32_t CircularBufferSize = 256u;
257-
class CAsyncQueue : public IAsyncQueueDispatcher<CAsyncQueue,SRequestType,CircularBufferSize>
267+
class CAsyncQueue final : public IAsyncQueueDispatcher<CAsyncQueue,SRequestType,CircularBufferSize>
258268
{
259269
using base_t = IAsyncQueueDispatcher<CAsyncQueue,SRequestType,CircularBufferSize>;
260-
//friend base_t;
270+
271+
core::smart_refctd_ptr<ICaller> m_caller;
261272

262273
public:
263274
inline CAsyncQueue(core::smart_refctd_ptr<ICaller>&& caller) : base_t(base_t::start_on_construction), m_caller(std::move(caller)) {}
264275

265-
void process_request(SRequestType& req);
276+
void process_request(base_t::future_base_t* _future_base, SRequestType& req);
266277

267278
void init() {}
268-
269-
private:
270-
void handle_request(SRequestType& req, SRequestParams_NOOP& param);
271-
void handle_request(SRequestType& req, SRequestParams_CREATE_FILE& param);
272-
void handle_request(SRequestType& req, SRequestParams_READ& param);
273-
void handle_request(SRequestType& req, SRequestParams_WRITE& param);
274-
275-
core::smart_refctd_ptr<ICaller> m_caller;
276279
};
277-
friend class ISystemFile;
280+
friend class ISystemFile; // TODO: do we need this friendship?
278281
CAsyncQueue m_dispatcher;
279282
};
280283

‎include/nbl/system/ISystemFile.h

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ class ISystemFile : public IFile
3131
params.file = this;
3232
params.offset = offset;
3333
params.size = sizeToRead;
34-
m_system->m_dispatcher.request(fut,params);
34+
m_system->m_dispatcher.request(&fut,params);
3535
}
3636
inline void unmappedWrite(ISystem::future_t<size_t>& fut, const void* buffer, size_t offset, size_t sizeToWrite) override final
3737
{
@@ -40,12 +40,13 @@ class ISystemFile : public IFile
4040
params.file = this;
4141
params.offset = offset;
4242
params.size = sizeToWrite;
43-
m_system->m_dispatcher.request(fut,params);
43+
m_system->m_dispatcher.request(&fut,params);
4444
}
4545

4646
//
47-
friend class ISystem::CAsyncQueue;
47+
friend struct ISystem::SRequestParams_READ;
4848
virtual size_t asyncRead(void* buffer, size_t offset, size_t sizeToRead) = 0;
49+
friend struct ISystem::SRequestParams_WRITE;
4950
virtual size_t asyncWrite(const void* buffer, size_t offset, size_t sizeToWrite) = 0;
5051

5152

0 commit comments

Comments
 (0)