-
Notifications
You must be signed in to change notification settings - Fork 8.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Give til::bitmap custom allocator support and add til::pmr::bitmap #8787
Conversation
til::details::bitmap<Allocator> will use Allocator for its dynamic_bitset, and it will use a rebound allocator for its run storage. Allocator should be an allocator type storing `unsigned long long`, the backing store type for dynamic_bitset. I've introduced a type alias, `til::bitmap`, which papers over the allocator choice for all existing code. I've also introduced a second type alias, `til::pmr::bitmap`, which lets a consumer use the C++ polymorphic allocator system.
src/inc/til/bitmap.h
Outdated
_sz(sz), | ||
_rc(sz), | ||
_bits(_sz.area()), | ||
_runs{} | ||
_bits(_sz.area(), fill ? std::numeric_limits<unsigned long long>::max() : 0, _alloc), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
functional change, but minor. instead of using set_all to fill all the bits, we are constructing the dynamic_bitset with 0xffffffffffffffff
stored in its blocks. It has the same effect!
I had to reindent the file because of namespace nesting. Review with whitespace hidden. |
Hello @DHowett! Because this pull request has the p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (
|
…icrosoft#8787) `til::details::bitmap<Allocator>` will use `Allocator` for its `dynamic_bitset`, and it will use a rebound allocator for its run storage. Allocator should be an allocator type storing `unsigned long long`, the backing store type for `dynamic_bitset`. I've introduced a type alias, `til::bitmap`, which papers over the allocator choice for all existing code. I've also introduced a second type alias, `til::pmr::bitmap`, which lets a consumer use the C++ polymorphic allocator system. I chatted with @miniksa about whether to keep the "full" allocator version in `details` or not. We decided that for the simplicity of the `til` namespace, we would. If anybody has a compelling reason to use `til::details::bitmap<Allocator>` directly, we can re-evaluate this decision.
* #8000 - Supports buffer rewrite work. A re-use of `til::rle` will be useful as a column counter as we pursue NxM storage and presentation. * #3075 - The new iterators allow skipping forward by multiple units, which wasn't possible under `TextBuffer-/OutputCellIterator`. Additionally it also allows a bulk insertions. * #8787 and #410 - High probability this should be `pmr`-ified like `bitmap` for things like `chafa` and `cacafire` which are changing the run length frequently. * [x] Closes #8741 * [x] I work here. * [x] Tests added. * [x] Tests passed. - [x] Ran `cacafire` in `OpenConsole.exe` and it looked beautiful - [x] Ran new suite of `RunLengthEncodingTests.cpp` Co-authored-by: Michael Niksa <miniksa@microsoft.com>
## Summary of the Pull Request Introduces `til::rle`, a vector-like container which stores elements of type T in a run length encoded format. This allows efficient compaction of repeated elements within the vector. ## References * #8000 - Supports buffer rewrite work. A re-use of `til::rle` will be useful as a column counter as we pursue NxM storage and presentation. * #3075 - The new iterators allow skipping forward by multiple units, which wasn't possible under `TextBuffer-/OutputCellIterator`. Additionally it also allows a bulk insertions. * #8787 and #410 - High probability this should be `pmr`-ified like `bitmap` for things like `chafa` and `cacafire` which are changing the run length frequently. ## PR Checklist * [x] Closes #8741 * [x] I work here. * [x] Tests added. * [x] Tests passed. ## Validation Steps Performed * [x] Ran `cacafire` in `OpenConsole.exe` and it looked beautiful * [x] Ran new suite of `RunLengthEncodingTests.cpp` Co-authored-by: Michael Niksa <miniksa@microsoft.com>
## Summary of the Pull Request Introduces `til::rle`, a vector-like container which stores elements of type T in a run length encoded format. This allows efficient compaction of repeated elements within the vector. ## References * #8000 - Supports buffer rewrite work. A re-use of `til::rle` will be useful as a column counter as we pursue NxM storage and presentation. * #3075 - The new iterators allow skipping forward by multiple units, which wasn't possible under `TextBuffer-/OutputCellIterator`. Additionally it also allows a bulk insertions. * #8787 and #410 - High probability this should be `pmr`-ified like `bitmap` for things like `chafa` and `cacafire` which are changing the run length frequently. ## PR Checklist * [x] Closes #8741 * [x] I work here. * [x] Tests added. * [x] Tests passed. ## Validation Steps Performed * [x] Ran `cacafire` in `OpenConsole.exe` and it looked beautiful * [x] Ran new suite of `RunLengthEncodingTests.cpp` Co-authored-by: Michael Niksa <miniksa@microsoft.com>
## Summary of the Pull Request Introduces `til::rle`, a vector-like container which stores elements of type T in a run length encoded format. This allows efficient compaction of repeated elements within the vector. ## References * #8000 - Supports buffer rewrite work. A re-use of `til::rle` will be useful as a column counter as we pursue NxM storage and presentation. * #3075 - The new iterators allow skipping forward by multiple units, which wasn't possible under `TextBuffer-/OutputCellIterator`. Additionally it also allows a bulk insertions. * #8787 and #410 - High probability this should be `pmr`-ified like `bitmap` for things like `chafa` and `cacafire` which are changing the run length frequently. ## PR Checklist * [x] Closes #8741 * [x] I work here. * [x] Tests added. * [x] Tests passed. ## Validation Steps Performed * [x] Ran `cacafire` in `OpenConsole.exe` and it looked beautiful * [x] Ran new suite of `RunLengthEncodingTests.cpp` Co-authored-by: Michael Niksa <miniksa@microsoft.com>
commit 4b0eeef Author: Leonard Hecker <lhecker@microsoft.com> Date: Fri May 14 23:56:08 2021 +0200 Introduce til::rle - a run length encoded vector ## Summary of the Pull Request Introduces `til::rle`, a vector-like container which stores elements of type T in a run length encoded format. This allows efficient compaction of repeated elements within the vector. ## References * #8000 - Supports buffer rewrite work. A re-use of `til::rle` will be useful as a column counter as we pursue NxM storage and presentation. * #3075 - The new iterators allow skipping forward by multiple units, which wasn't possible under `TextBuffer-/OutputCellIterator`. Additionally it also allows a bulk insertions. * #8787 and #410 - High probability this should be `pmr`-ified like `bitmap` for things like `chafa` and `cacafire` which are changing the run length frequently. ## PR Checklist * [x] Closes #8741 * [x] I work here. * [x] Tests added. * [x] Tests passed. ## Validation Steps Performed * [x] Ran `cacafire` in `OpenConsole.exe` and it looked beautiful * [x] Ran new suite of `RunLengthEncodingTests.cpp` Co-authored-by: Michael Niksa <miniksa@microsoft.com>
## Summary of the Pull Request Introduces `til::rle`, a vector-like container which stores elements of type T in a run length encoded format. This allows efficient compaction of repeated elements within the vector. ## References * #8000 - Supports buffer rewrite work. A re-use of `til::rle` will be useful as a column counter as we pursue NxM storage and presentation. * #3075 - The new iterators allow skipping forward by multiple units, which wasn't possible under `TextBuffer-/OutputCellIterator`. Additionally it also allows a bulk insertions. * #8787 and #410 - High probability this should be `pmr`-ified like `bitmap` for things like `chafa` and `cacafire` which are changing the run length frequently. ## PR Checklist * [x] Closes #8741 * [x] I work here. * [x] Tests added. * [x] Tests passed. ## Validation Steps Performed * [x] Ran `cacafire` in `OpenConsole.exe` and it looked beautiful * [x] Ran new suite of `RunLengthEncodingTests.cpp` Co-authored-by: Michael Niksa <miniksa@microsoft.com>
til::details::bitmap<Allocator>
will useAllocator
for itsdynamic_bitset
, and it will use a rebound allocator for its run storage.Allocator should be an allocator type storing
unsigned long long
, thebacking store type for
dynamic_bitset
.I've introduced a type alias,
til::bitmap
, which papers over theallocator choice for all existing code. I've also introduced a second
type alias,
til::pmr::bitmap
, which lets a consumer use the C++polymorphic allocator system.
I chatted with @miniksa about whether to keep the "full" allocator
version in
details
or not. We decided that for the simplicity of thetil
namespace, we would. If anybody has a compelling reason to usetil::details::bitmap<Allocator>
directly, we can re-evaluate thisdecision.