Skip to content

Generalize arrangements to Containers#281

Merged
frankmcsherry merged 5 commits intoTimelyDataflow:masterfrom
frankmcsherry:columnation
Nov 24, 2022
Merged

Generalize arrangements to Containers#281
frankmcsherry merged 5 commits intoTimelyDataflow:masterfrom
frankmcsherry:columnation

Conversation

@frankmcsherry
Copy link
Copy Markdown
Member

This PR-in-progress generalizes the storage used for arrangement batches to a Container trait that informally looks like a Vec but is not obliged to have that implementation. In particular, we are interested in the Columnation implementation, which maintains a Vec-like storage but whose items can arrange their owned members in contiguous memory.

frankmcsherry and others added 2 commits August 26, 2022 12:44
generalized implementation

generalize ord to containers

very silly error

prototype column container

demonstrate potential improvement

tabs to spaces

enable regions in batch formation

fix test
…imely

Signed-off-by: Moritz Hoffmann <antiguru@gmail.com>
Comment thread src/trace/layers/mod.rs Outdated
Comment on lines +165 to +172
fn with_capacity(_size: usize) -> Self {
Self::default()
}
fn reserve(&mut self, _additional: usize) {
}
fn merge_capacity(_cont1: &Self, _cont2: &Self) -> Self {
Self::default()
}
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These look like they should be implemented in order to be competitive.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comment thread src/trace/implementations/ord.rs Outdated
let mut write_position = index;
for position in index .. self.len() {
if predicate(position, &self[position]) {
// TODO: compact the inner region and update pointers.
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What does this mean? :D

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks vestigial, as it also appears (with clearer meaning) in the columnation code.

Comment thread src/trace/implementations/ord.rs Outdated
Comment thread src/trace/implementations/ord.rs Outdated
antiguru and others added 2 commits September 6, 2022 14:22
Signed-off-by: Moritz Hoffmann <antiguru@gmail.com>
Expose capacity/reserve of TimelyStack
@frankmcsherry frankmcsherry marked this pull request as ready for review November 24, 2022 15:25
Comment thread src/trace/layers/mod.rs
Comment on lines +169 to +170
fn reserve(&mut self, _additional: usize) {
}
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We use this in various push_merge implementations for trie layers. I don't think it should block merging this PR, but probably worth pondering at least briefly. For example:

        let (trie1, mut lower1, upper1) = other1;
        let (trie2, mut lower2, upper2) = other2;

        self.vals.reserve((upper1 - lower1) + (upper2 - lower2));

@frankmcsherry frankmcsherry merged commit 93da913 into TimelyDataflow:master Nov 24, 2022
This was referenced Oct 29, 2024
@frankmcsherry frankmcsherry deleted the columnation branch September 23, 2025 23:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants