Clean up CompositionSet and Context interfaces

[tom-kuchler]

Current situation:

The CompositionSet currently accessed by the dispatcher and the multinode code via the implementation of IntoIterator trait.
Calling into_iter (or just using for _ in <CompositionSet>) will result in an iterator with the item type usize, usize, Arc<Context>, which represent the set index, item index and the context from which to get the item.
The reason for this, is that the indices are need for the contexts transfer function.
Currently the composition set holds on to a set index, as well as a vector of tuples containing keys, item indices and the context references.

Possible change:

Simplify processing by converting the content of the Context into the CompositionSet.
By this I mean, take the Contexts content vector and allow the CompositonSet to directly own the item vector.
For each item we store it as a tuple with a Arc<Context> as we do now.
This simplifies the iterator, as it can simply iterate over the item context tuples, allowing easy access, with less unwraps etc.
We can store a Arc<Context> with each item, so we can still merge CompositionSets from different contexts, and we can still sort by item key, preserving all the Sharding iterators etc.

Possible follow up:

Since we would have each item hold the context only to access a specific range of it's context, we could think about partial release of contexts.
By this I mean, that when we convert a context into it's composition sets, free the parts of the context not owned by any of the items, and free space used by items, as they are dropped individually.

[tom-kuchler]

@tostocker what do you think about this idea?
I found it a bit annoying when working on the multinode serialisation, that iterating through them is quite dumb, especially considering it should be fairly straight forward.
I think there are a few more improvements, that could be made in the context interfaces etc., but this seems low hanging fruit.

[tostocker]

I'm not sure if I fully understood what you're proposing, but if I understand it correctly you want to have the CompositionSet contain what is currently stored in the content vector of the Context struct instead of having indexes in the CompositionSet to that content vector.

This sounds reasonable to me, also since I never really understood why we had two data set notions and that would simplify it to one. If I understood your proposal correctly we would then have some sort of Position into the Arc<Context> which I think could also be beneficial for better sharding policies that take the set/item size into consideration (which we would currently have to get via the context content using both set and item index).

[tom-kuchler]

Yes that is what I was thinking.
The reason it was originally done with indices, was that we keep all metadata in one place in the context, to make sure if we need it to cross reference other items or something, it would be available.
Since that has not come up and looks strongly like it will never come up, I think we can simplify the design to make future changes a bit easier.
Then I'll go ahead and propose some changes. I opened a PR #104 with a version I think looks reasonable.

[tostocker]

After discussing this in detail we came to the conclusion that we can further decouple the data sets/items from the context in the future. This is a bit more involved which is why we will leave it for future work.

Possible Future Changes

The DataSet and DataItem structs are only used in the content vector of the Context which is used to setup the engine environment for the function invocation and is later replaced with the function outputs. At the same time we use the CompositionSet` struct for all the scheduling in the dispatcher.

Decoupling the DataSet and DataItem structs from the Context removes the need for a separate CompositionSet struct and instead of having a content vector in the Context we can give the DataSet instances to the engine for execution and get back a vector with the output DataSets instances.

This could also simplify future memory optimizations like keeping only the output sets and not the entire function context around after execution.

I can think of refactoring the structs to look something like:

pub struct DataItem {
    pub ident: String,
    pub key: u32,
    pub size: usize,
    pub data: (Arc<Context>, usize), // a tuple with context reference and offset
}

pub struct DataSet {
    pub ident: String,
    pub size: usize, // set size for scheduling policies
    pub buffers: Vec<Arc<DataItem>>, // arc so we can do sharding more easily
}

pub struct Context {
    pub context: ContextType,
    pub state: ContextState,
    pub size: usize,
    occupation: Vec<Position>,
}

// can get rid of CompositionSet struct

[tostocker]

AutoSharding minimum set size

The AnyJoinIterator currently only considers the min_set_bytes of the largest set after joining if it inner/outer/left/right joins two or more sets. This is very handwavy, thus when solving this issue, we should also come up with a better way to handle this in the future.