- use uniform naming scheme
- extract type-binding from RangeIter into a trait-template
- use the C++20 iterator concept and trait to detect a STL iterator
After some investigation, I seem to understand the problem better now.
This builds upon the Introduction of our `ValueTraits´ two years ago:
e176e54004
The root cause to these ongoing sporadic problems with type deduction
seems to be that we encounter a **clash of concepts**: What Lumiera calls
an »Iterator« is essentially different to the order of terms used by the STL.
In the STL, an iterator can be seen as an ''abstracted pointer'', and is
thus only meaningful in conjunction with a backing container. It is also
this backing STL container that provides the value-type definitions.
In Lumiera however, an ''Iterator is a promise to retrieve a sequence of evaluations''.
For that reason, the Iterator itself provides value-type definitions,
and the notion of "iterable" can largely be conflated with "Lumiera Iterator".
Now, the `lib::RangeIter` can act as a bridge between both usage patterns,
and thus it is possible to create a `RangeIter` both from a Lumiera Iterator
or from a STL container. (As an aside, this is a similar idea as promoted
by the "Range TS" in C++20 -- yet the Lumiera iterator concept is much more
abstract and allows evaluations like tree expansion and search algorithms,
which would be never possible with the STL based notion of iteration)...
Thus the value type binding in `RangeIter` is the only correct place
where to mitigate this ''clash of concepts:''
* if the `IT` parameter is a Lumiera iterator, we need to retrieve
the type bindings defined therein
* otherwise, we can assume that `IT` is an STL iterator and thus
essentially a pointer, and we must strip that pointer and work from there.
...to emplace some object into the buffer space.
I wouldn't expect that to be necessary for the real usage,
but it comes in handy for tests, where we often want to place
some sentinel value into a buffer.
- visible Signature of TestFrame changed due to being relocated into the Vault
- attempt to make the WorkForce_test more robust;
frequently fails on testsuite run after full build;
so better ''wait'' for the workers to have started working,
instead of using a fixed delay (that must be extremely short
and is hard to predict)
Straight-forward:
- add a map keyed by observed frame numbers
- individual connection needs to know its own seqNr :-(
- also need to store and recall the frameNr in the buffer-Index
Fix bug with DataSink constructor
(value constructor shadows copy constructor => Stackoverflow)
''Complete construction and wiring sequence works as intended''
- can now throw away the old convoluted `TrackingInMemoryBlockSequence`
- instead use a simplistic implementation of the Connection interface
- do not perform any kind of validation, allocate new buffer for each frame
- use the `NaiveBufferSetup` (`BufferProvider`) as implementation backend.
This will be the first ''reality test'' for the new `OutputSlot` internal design:
Is it possible to implement a tracking test implementation in a clean
and straight-forward way, avoiding all the tangling and indirection
that plagued the old prototype implementation from 2012?
After resolving the inner tensions in the old prototype from 2012,
it was now possible to resolve all issues in a single strike
* `DataSink` becomes a functor, yet embeds a shared ref-count
* in the λ-closure a ''specific Buffer Descriptor'' is prepared for each feed
* building upon the prototyping work, a dedicated `OutputBufferProxy`
exposes a `BuffHandle` to the client, with `emit()` and `release()` calls
* the implementation ''delegates directly'' to the `OutputSlot::Connection`
* an this is where all actual output technology is encapsulated
...this is the gordian λ
Note
- embedding a copy of the shared-ptr in the closure
- pre-configure a buffer descriptor (buffer type)
* with the buffer size as retrieved from the connection
* and pass the connection itself marked as LocalTag
Thus, on invocation of the DataSink (functor), it suffices
to invoke the lock() function on the BufferDescriptor,
using the new additional argument introduced with the
preceding commit
Reaping the benefits of the preceding refactorings, most of the complex
wiring within `OutputBufferProxy` can be omitted, and the proxy calls
can be delegated directly to the actual `Connection`
There is a catch however: we need to create an additional pathway
to feed the frame-number information into the `BufferStore` backend.
The solution adopted here is the result of extensive reflection.
Taken literally, or naively, the reworked architecture would imply
to route every call through a delegation-chain that is dispatched
in the state-managing "PImpl" (`AllocState<CON>`).
However, in keeping with the idea of an opaque functor embedded into
`DataSink`, direct and explicitly typed calls can be bound in the
associated closure. Doing so will strengthen the role of the
interface `Connection` — which defines the boundary to the
actual domain of output technology.
This changeset attempts to breach into the implementation design of OutputSlot
(while largely retaining the API and the »Output Slot Protocol«).
While the actual problem that brought me to this point can be traced back to some
rather obvious mistake, namely to conflate the `BufferProvider` used as
backing implementation with a `BufferProvider` used as front-end API,
creating the new connections to use an `OutputBufferProxy` unveils some
further long-standing structural mismatch; notably this leads to the
difficulties to get the lifecylce management of `OutputSlot` and `DataSink`
under control.
So I remove one entire level from the implementation,
in a bold move to realign the design with the prospective usage pattern.
Obviously, this breaks a lot of further code, and I need to rewire
the Allocation implementation altogether, and provide a new functor based
implementation for `DataSink`
After a lot of pondering over various solution schemes,
it seems that `BufferProxyAdaptor` is too rigid for the task at hand.
It could be retained as a technology demonstration, and used as blueprint
for a custom-made implementation. In a nutshell, we need a `BufferStore`
that directly delegates to the respective `Connection` — which also implies
to use the `OutputSlot::AllocState` as an intermediate to hendle that delegation.
...which still looks like a nasty task
* the good part is, we have a working `BufferProxyAdaptor` now
* yet this setup does not seem to fit into the `OutputSlot::AllocState`
As an aside, several test definitions need to be relocated to the Vault,
since the whole compound around `BufferProvider` migrated down
This is a nasty new requirement, discovered as an aside while looking
into ways to resolve the ''crucial mismatch'' between the `OutputSlot`
and `BufferProvider`, using the proxy-implementation of `BufferProvider`
(thereby expanding on the recent draft with the `BufferProxyAdaptor`)
The point is, if the BufferMetadata will be used beyond the internal coordination
within BufferProvider, then it will be used as part of actual buffer storage.
And this implies that the actual size of the allocation may differ (be greater)
than the nominally required size (since we'll very likely employ some kind
of memory tiling, for reasons of performance).
This leads again to an analysis of `BufferMetadata` usage,
with the conclusion to add the actual storage size directly into the API,
but treat it as an ''extra'' and not integrated into the ''buffer type''
Phew ... quite complex alltogether
This might have seemed like a good idea, adding an additional ''consistency check''...
However — we ''do have'' a generic check, insofar `BufferMetadata` is a **state engine**.
As long as we're able to prevent, reliably, that the same buffer address
ends up being classified as a different buffer type, this should be enough
to prevent double-use of any buffer (letting aside that `BufferProvider` is
backed by an implementing memory allocator, which in itself should prevent
any double-use.
Removing this feature resolves some ambiguity in the overall design,
and allows to turn `BufferMetadata::lock(type, address)` into a private
implementation function.
In the old code, there were some external access paths directly entering
into that implementation function, which caused a lot of confusion (at least for me...)
This refacotring prepares for the planned changes in `OutputSlot`
* introduce a new compound parameter (tuple) BufferAlloc
* wire this through all calls involved with storage handling
__Background__:
* `BufferMetadata` deemed important for the implementation of buffer storage
* ''new requirement detected'': must be able to handle an actual storage block
that is ''larger'' than the requested buffer size (⟼ tiling)
* need compact and uniform info, also for the `BufferProxyAdaptor` callbacks
There was an old UML diagram created at the time of the first draft.
It is integrated into the »OutputSlot« page in TiddlyWiki. This description
and the diagram are highly relevant for understanding the design in this case.
Unfortunately, Umbrello was "improved" with a lot of features since I used it
for the last time, but the **quality** of the software has been **degraded**,
it is now buggy and the layout is imprecise and gets messed up easily.
__Warning__: the old diagrams look ''slighly off alignment'',
this is very likely due to some change in font handling.
I have set the default font now to 'DejaVu Sans Condensed' 9pt
The old implementation relied on two layered APIs:
one for the client and one for the implementer of
the Allocation state. This is logically sound, yet
clumsy in terms of design, since the internal API
turns out to be void of actual meaning.
It seems possible to eliminate this confusing structure
by a clear separation of ''implementation realms''
* the ''mechanics'' of connection state management
* the actual implementation of the connection
The first can be provided as generic template subclass,
while the latter should be confined entirely into the
implementation of the `Connection` interface.
Doing so requires to keep each part of the implementation
strictly focused onto its purpose — something I obviously
was not able to see at the time when writing this old
implementation, notably because I was completely guided
and driven by the test/dummy implementation without
considering how an actual implementation of
low-level data output would work.
This API function was the reason why the handling of
`OutputSlot` and `BufferProvider` do not mesh up well,
so that the former could not yet be integrated properly
into the engine.
''Obviously the proper solution still needs to be implemented''
The state of connectedness is represented as class,
which actually is a three step deep hierarchy.
This design seems plausible, since we get
- an interface visible for clients (`Allocation`)
- an internal interface
- the actual implementation, templated to the actual connection
At least we can try to choose names that better indicate
these three related levels.
The old implementation of `DiagnosticOutputSlot` is now disabled
and will be re-written from scratch, and no longer tangled to `BufferProvider`
- `OutputManager` remains in Steam-Layer for now
- the `Timings` and `OutputSlot` are now a self-contained
One of the goals of this extended clean-up and refactoring is to allow
access to the data buffers of a `DataSink` through a `BuffHandle` as front-end.
Which essentially means that the ''Buffer Provider Protocol'' and the
''Output Slot Protocol'' mesh together well.
After the clean-up of the `BufferProvider` implementation hierarchy,
the viability of this idea was demonstrated with the `BufferProxyAdapter` draft;
so it is now roughly clear what must be changed within the existing implementation
of `OutputSlot`. More specifically, the only actual implementation ever completed
was a dummy implementation for test (`DiagnosticOutputSlot`), which hooked
confusingly into the corresponding dummy implementation of `BufferProvider`,
that was written one year earlier (and was also without actual implementation
at that time).
So the second half of the task ahead seems very similar to the first part:
- the base implementation of the ''Output Slot Protocol'' seems roughly acceptable
- but the actual dummy implementation was somewhat confused and should be
re-written from scratch, without ''any attempt to cut corners by re-using other dummy code''
__Note__ as part of this analysis I discovered some gap in the design
regarding timeout handling ⟶ Ticket #1417
building upon all the preparations done during the last days,
the compound of code connected to the `BufferProvide` can now
be migrated down into the vault:
- `BuffHandle`
- `BufferProvider`
- `BufferMetadata`
- `BufferProviderSetup`
- the simple standard implementations
- `DiagnosticBufferProvider`
- all directly related unit tests
Gradually, the header buffhandle.hpp became the minimal include
to use basic `BufferProvider` functionality, since `BuffHandle`
provides proxy functionality to lock and access a data buffer.
The whole compound connected to the `BufferProvider` has evolved
into a cohesive yet central connection hub related to buffer memory;
it should thus be extracted, moved down to »Vault« layer and used
4as a foundation for various high-level tasks. In preparation of
this step, relevant marker typedefs are re-arranged;
notably ''Stream Type'' now includes `buffhandle.hpp`
These are generic vehicles to verify computation structures.
Since they are in no way related to our models, they belong into »Vault« layer
Furthermore this reorganisation of test support components is prerequisite
to also move further generic facilities down into the Vault
This resolves a long-standing issue:
we could not use proper include paths for test-helper code that was defined alongside the tests.
Since some test-helper code must now be relocated into the »Vault«-Layer,
I took the opportunity to finally resolve this annoyance. The solution is two-fold
* introduce a dedicated test-support library
* remove the quirks that were necessary to support some plain-C tests
Together, this allows to raise the include-directory by one level,
and then to adjust all folder names so that they match the namespace
This is something I wanted to clean-up since a very long time indeed,
but always postponed it, since it requires a lot of nasty clean-up and disentangling....
Several parts had been stashed away into `src/lib/test`, which are not really related:
* the testrunner and testsuite builder
* generic helper utilities
* specialised test-support facilities, e.g. the EventLog
This changeset introduces a new top-level test folder and a namespace `::test`
Code from this folder is linked into a shared library `libtestsupport.so`,
which resides in the linkage hierarchy between »Vessel« and »Vault«,
yet is no longer part of the Lumiera application and only linked into the test-suite.
The next step with the `BufferProvider` and `OutputSlot` refactoring
will require to create new definition headers and to move those self-contained
services down into the »Vault« layer. In preparation, I decided deliberately to
abandon that old idea of a ''minimal bootstrap'' of arbitrary components in
favour of building the application as a coherent whole, with well-defined
boundaries and a backbone structure, marked as such explicitly by name
and arrangement of the code.
The changes were documented and applied to the integration branch first,
and now it's time to join that effort with the ongoing work on `BufferProvider`.
During the last months, a lot of gradual changes to policies and guidelines
had been finalised and documented. Moreover, recent clarifications regarding
memory management in the render engine will lead to a clearer definition
of the »Vault« Layer's role, with some ''engine code'' expected to move down.
On this occasion, I reworked relevant sections in the documentation to
confirm the role that now falls to central parts of the overall architecture.
Notably I decided to rename those parts related to the application backbone
as »Vessel« — to underpin the more active and defining role they assumed.
Explored further aspects of this new design;
as an aside, I tried to build a Concept to describe the policy,
which howerer turned out to be much more challenging than expected.
Lastly, I could now complete the prototype of `BufferProxyAdaptor`
While it is trivially simple to define some struct with callback functions,
such a definition turns out to be obnoxiously complex and hard to read in pracice,
thereby nullifying any benefit from the shorthand λ-based notation.
This kind of problem with the readability at client-side is quite common
when using callbacks for a policy-based design. To overcome this difficutly,
it would be necessary to define the policy and the callbacks in one
succinct builder expression. Unfortunately such a builder syntax
typically requires to write a lot of boilerplate template code.
After several failed experiments, I have found a bold new way to
exploit features of C++20 in conjunction with a macro, allowing
to generate a suitable data record directly form an C++ expression.
The idea is to use a "transient" Setup helper class, which lives
only during the invocation of the BufferProviderSetup-constructor.
This transient frame allows to accept a policy class from the client,
and move it through this transient Setup helper and finally into
the BufferStore backend implementation, where it is actually needed.
Now I have a working setup for a `BufferProxyAdaptor`.
Unfortunately this is still confusingly complex.
This is due to my intention to build a generic template,
that can be configured with a policy to invoke arbitrary callbacks,
whenever one of the passed-through buffers transitions through its lifecycle.
* we need to configure the `BufferProviderSetup` to use our specific implementation classes
* and the second one of these, our `BufferStore` implementation needs to invoke the callbacks
This closes a ''conceptual gap.''
There was a `lib::TestFrame::reseed()` function, which should be invoked
whenever a clean / deterministinc seed is required for computations involving
`TestFrame` data.
On occasion I noticed that most actual usages of `TestFrame`, which became quite common
lately, were missing this `reseed()` call, while the enclosing test
had indeed a proper Test::seedRand() call.
So my conclusion is: if even ''I myself'' miss that fine point frequently,
it is justified to automated this setup, even while `TestFrame` is right now
the only client of such a setup-facility.
There is a twist though.
`TestFrame` is part of the ''Engine'' test coverage, i.e. it is linked
into `libtest-core.so`. While the support for random numbers and reseeding
is obviously a very basic library feature, and linked into `liblumiera.so`
**Oh well**.
So now we have ''yet another'' static registration service.
I kept everything as simple as possible, which means, we continue
to use the ''common, global random generator'' from the library,
and there is no protection against some other code using those
library functions in a non-deterministic way.
`lib/random-reseed.cpp` is just a glorified hard-wired setup, beware!
This is an implementation experiment to validate the improvements
and internal clean-up. Previously, I was unable to implement such a
forwarding proxy buffer provider, as would be required for the
intended harmonisation of the `OutputSlot` interface with `BufferProvider`
With the reworked verison of the BufferProvider standard implementation
it seems straight-forward to...
* define a »Buffer Type« that marks any of the prepared buffers to pass-through
* use the resulting Buffer Descriptor together with a rigged storage backend,
that just returns the ''local tag'' as buffer adress
The first round of the [ticket:1410 BufferProvider clean-up] is roughly complete.
* a much cleaner diagnostic interface has been defined and implemented
* this allowed to modernise the `DiagnosticBufferProvider_test`
and the `BufferProviderProtocol_test`
* the implementation scheme has been split in two realms
* the old heap-memory based implementation has been re-arranged,
so that the aforementioned tests are **green** again.
Now is the point to draft a solution to get the interfaces of
`BufferProvider` and `OutputSlot` aligned, so that the latter
can expose its lifecycle through the former.
Today, I have analysed various aspects and developed a pathway
to get into that task, and next I will reshape the previously
failed `OutputProxyProvider_test` into a prototype to
validate this idea
* write down some first considerations regarding requirements
of the actual production-strength implementation of buffer management
* consider relevance of the part of the API that was retained
* write a new series of test cases to cover handling of embedded objects,
using the test support for tracking object creation
Some weeks ago, at the time where I started to dig into this topic,
I extended the BufferProviderProtocol_test to perform a buffer usage pattern
that mimics the usage in the real Render Engine, as implemented last year.
For this, I used the `TestFrame` and the chained-hash dummy computations.
Now, after reordering and clean-up of the `BufferProvider` implementation interfaces,
it can be showed with this computation scheme that
- all buffers are indeed allocated and handed out as expected
- the metadata handling is correct and points to the processed buffers
- the complete memory used in this test is uncorruped and holds correct numbers
It covers now roughly the same procedures as before,
yet the scheme how to do verification is quite different.
Furthermore, we allocate large random-sized buffers now
and implant `TestFrame` instances, and so also the verification
covers a significant storage area.
* can now replace old verification code by new diagnostic functions
* test case runs without failure
''UNRESOLVED:'' failure in verification of memory contents
The drafted setup for capturing tracking information was indeed way to elaborate.
Notably I wanted to capture the actual `BuffHandle` -- and that caused a host
of visibility and access problems, since that handle was meant to be used
as a front-end to the BufferTracker, and only that.
Now we just capture the hash-IDs and the storage size,
and we do not even keep relations or record a state-ENUM;
we just smash a copy of that data into a registry vector
for each state separately, which allows to do a lookup,
count the number of handled allocations and access the memory.
...this turns out to be surprisingly difficult,
due to various subclass / protected visibility problems.
Not sure if the component design within `DiagnosticBufferProvider`
is sane yet...
Use a generic helper function in the `BufferProviderSetup` base class
to allow layering of a decorator implementation on top of an already
instantiated implementation class.
After the preceding internal reorganisation, it suffices
for the `DiagnosticBufferProvider` to instrument the `BufferStage` interface,
in order to capture and record all relevant buffer state transitions
This is clearly implementation code and can now be removed from the interfaces;
`SimpleBufferStateRegistry` can thus delegated to a private metadata registry
In the end I refrained from applying most of the changes I had intended.
I am aware that `BufferProvider` is still quite a massive structure,
yet with the implementation refactorings done thus far, it seems
to be reasonably clear and self-contained now.
It would be possible to totally overthrow the grown structure and
expose only the two handles (BuffDescr and BuffHandle), together with
a factory function, but I cannot see how this would help to make the
code clearer and more approachable. Maybe it would make the usage
more obvious from the client's perspective — yet, to be honest,
there are not ''that much clients'' to justify such a move.
Furthermore, the implementation is far from finished: for both of
the back-end lines, a fully production-strength implementation
must be created, that is able to operate in a highly concurrent
environment; very likely some lock-free messaging and hand-over
between threads will become necessary, adding to the complexity.
Thus it seems better to note the ''design critique'', but to
postpone any further refactorings to a point where we can watch
a fully developed Render Engine under production load.
doing so requires to trace a new call path from front to back,
into the `BufferMetadata` implementation: there is now an explicit check
that the buffer is in `LOCKED` state, but not yet `EMITTED` and als that
it was not yet `released()`
I think the additional effort is justified
* in the regular / currently intended usage in the Render Engine,
this predicate is not required, since compliance to the ''Buffer Provider Protocol''
as such is sufficient to ensure that
* but since all of this is getting more generic and polished now,
I could imagine future usages beyond the current confines; and then,
this semantics is what you'd expect from such a `bool()` check:
»I am allowed to use it«
