Give til::bitmap custom allocator support and add til::pmr::bitmap #8787

DHowett · 2021-01-14T01:11:07Z

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.

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.

DHowett · 2021-01-14T01:15:34Z

src/inc/til/bitmap.h

            _sz(sz),
            _rc(sz),
-            _bits(_sz.area()),
-            _runs{}
+            _bits(_sz.area(), fill ? std::numeric_limits<unsigned long long>::max() : 0, _alloc),


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!

DHowett · 2021-01-14T01:22:11Z

I had to reindent the file because of namespace nesting. Review with whitespace hidden.

ghost · 2021-01-19T18:24:27Z

Hello @DHowett!

Because this pull request has the AutoMerge label, I will be glad to assist with helping to merge this pull request once all check-in policies pass.

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 (`@msftbot`) and give me an instruction to get started! Learn more here.

@miniksa

…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>

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>

DHowett commented Jan 14, 2021

View reviewed changes

DHowett changed the title ~~Add custom allocator support to til::bitmap~~ Give til::bitmap custom allocator support and add til::pmr::bitmap Jan 14, 2021

SA + Format

3e732a4

miniksa mentioned this pull request Jan 15, 2021

til::rle<T, S> - a run length encoded storage template #8794

Closed

6 tasks

zadjii-msft assigned miniksa Jan 15, 2021

miniksa approved these changes Jan 15, 2021

View reviewed changes

DHowett added the Needs-Second It's a PR that needs another sign-off label Jan 18, 2021

ghost requested review from zadjii-msft, carlos-zamora, leonMSFT and PankajBhojwani January 18, 2021 19:22

DHowett mentioned this pull request Jan 18, 2021

Eliminate more transient allocations: Titles and invalid rectangles and bitmap runs and utf8 conversions #8621

Merged

6 tasks

carlos-zamora approved these changes Jan 19, 2021

View reviewed changes

DHowett added the AutoMerge Marked for automatic merge by the bot when requirements are met label Jan 19, 2021

ghost merged commit 2919d96 into main Jan 19, 2021

ghost deleted the dev/duhowett/bitmap-with-allocator-magic branch January 19, 2021 18:24

lhecker mentioned this pull request May 14, 2021

Introduce til::rle - a run length encoded vector #10099

Merged

6 tasks

This pull request was closed.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Give til::bitmap custom allocator support and add til::pmr::bitmap #8787

Give til::bitmap custom allocator support and add til::pmr::bitmap #8787

DHowett commented Jan 14, 2021 •

edited

Loading

DHowett Jan 14, 2021

DHowett commented Jan 14, 2021

ghost commented Jan 19, 2021

Give til::bitmap custom allocator support and add til::pmr::bitmap #8787

Give til::bitmap custom allocator support and add til::pmr::bitmap #8787

Conversation

DHowett commented Jan 14, 2021 • edited Loading

DHowett Jan 14, 2021

Choose a reason for hiding this comment

DHowett commented Jan 14, 2021

ghost commented Jan 19, 2021

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 (@msftbot) and give me an instruction to get started! Learn more here.

DHowett commented Jan 14, 2021 •

edited

Loading

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 (`@msftbot`) and give me an instruction to get started! Learn more here.