Repository navigation
Conversation
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
Azure Pipelines: There may be pipelines that require an authorized user to comment /azp run to run. |
Contributor
Author
|
/azp run |
|
Azure Pipelines: Successfully started running 1 pipeline(s). |
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
Author
|
/azp run |
|
Azure Pipelines: Successfully started running 1 pipeline(s). |
| - Allocate an index for usage (fails if already allocated) | ||
| - Release an index (by default, when all indexes in a page are released, the page is freed) | ||
| - Get an index (fails if not allocated) | ||
| - Free the pages that contain the indexes of a range, if no element in those pages is allocated |
| - `page`: a pointer to the page (see [Page Structure](#page-structure)), `NULL` when the page is not allocated | ||
| - `use_count`: the count of allocated elements in the page plus the count of calls that use the page at that moment. While the page is being freed, `THREAD_SAFE_PAGED_SPARSE_ARRAY_PAGE_FREEING` (2<sup>62</sup>) is added to it. | ||
| - `lock`: an `SRW_LOCK_LL` that is acquired in exclusive mode only to allocate or to free the page | ||
|
|
There was a problem hiding this comment.
Are these explanations just duplicating the underlyiong PAGED_SPARSE_ARRAY? I don't think we should duplicate but rather explain that this is the thread safe one that wraps the underlying non thread safe thingie.
I hope that's what we want, right?
|
|
||
| | Option | Value | Meaning | | ||
| |---|---|---| | ||
| | `empty_pages` | `AUTO_FREE_EMPTY_PAGES` (default) | A page is freed when its use count becomes 0: no element in it is allocated and no call uses it. | |
| |---|---|---| | ||
| | `empty_pages` | `AUTO_FREE_EMPTY_PAGES` (default) | A page is freed when its use count becomes 0: no element in it is allocated and no call uses it. | | ||
| | `empty_pages` | `KEEP_EMPTY_PAGES` | Empty pages are kept until `FREE_EMPTY_PAGES` is called or the array is disposed. This avoids freeing and re-allocating a page when its elements are released and allocated again in quick succession (e.g. when the array is used as a ring buffer). | | ||
| | `new_pages` | `DO_NOT_ZERO_NEW_PAGES` (default) | The items of a newly allocated page are left uninitialized. | |
There was a problem hiding this comment.
Aren't these 2 mutually exclusive?
| The rules for callers are: | ||
| - Only the user that allocated an `index` shall release it, and an `index` shall not be released more than once per allocation. | ||
| - The array only protects its own bookkeeping. Synchronizing access to the contents of an element (e.g., one thread writing an element while another thread reads it after `GET`) is the responsibility of the caller. | ||
| - `item_dispose_func` is called without holding any lock. |
There was a problem hiding this comment.
samrach9 Did you read all of this document?
| **SRS_THREAD_SAFE_PAGED_SPARSE_ARRAY_09_010: [** `THREAD_SAFE_PAGED_SPARSE_ARRAY_CREATE(T)` shall succeed and return a non-`NULL` value. **]** | ||
|
|
||
| **SRS_THREAD_SAFE_PAGED_SPARSE_ARRAY_09_011: [** If there are any other errors, `THREAD_SAFE_PAGED_SPARSE_ARRAY_CREATE(T)` shall fail and return `NULL`. **]** | ||
|
|
There was a problem hiding this comment.
Seems like this is not using the existing implementation underneath?
I think Dan's point about SRP in the last iteration was that PAGED_SPARSE_ARRAY should be used as is and the thread safety should be provided on top rather than in the same module.
Member
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds a new module,
THREAD_SAFE_PAGED_SPARSE_ARRAY(T): a paged sparse array whoseALLOCATE,RELEASE,GETandFREE_EMPTY_PAGEScan be called concurrently from multiple threads. The existingPAGED_SPARSE_ARRAYis unchanged; the new module reuses its result codes (PAGED_SPARSE_ARRAY_ALLOCATE_RESULT,PAGED_SPARSE_ARRAY_GET_RESULT) and bitmap helpers.This is the c-util part of PBI 40316555, which replaces the fixed, preallocated sequence-indexed arrays in EBS
bs_ordered_op_writer(~40 MB per primary) andbsi_sequence_tracker(~4 MB) with a structure that sizes itself to the operations in flight. Moving the writer and the tracker to it will be separate EBS PRs.ALLOCATE,RELEASEandGETon an allocated page take no lock. They only use interlocked operations on the page's use count and allocation bitmap.ALLOCATEinto it) or freed. Calls on other pages never wait for it.ALLOCATE/GETstays valid until that index is released.FREE_EMPTY_PAGES(start_index, end_index): frees the empty, unused pages that contain any index in the range, and skips the others.THREAD_SAFE_PAGED_SPARSE_ARRAY_TYPE_DEFINE(T)usesAUTO_FREE_EMPTY_PAGESandDO_NOT_ZERO_NEW_PAGES.THREAD_SAFE_PAGED_SPARSE_ARRAY_TYPE_DEFINE_WITH_OPTIONS(T, KEEP_EMPTY_PAGES, ZERO_NEW_PAGES)selects the others.KEEP_EMPTY_PAGESis for ring buffers: the caller frees pages when its tail leaves them, instead of a page being freed and re-allocated every time it becomes empty. A misspelled option, a number, or swapped options do not compile.FREE_EMPTY_PAGES, and a producer/consumer ring buffer). Passing on Windows and Linux; traceability and repo validation pass.