-
Notifications
You must be signed in to change notification settings - Fork 5
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
only allocate memory if needed #50
only allocate memory if needed #50
Conversation
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.
this will not work as intended.
please let me think about this for a second I believe there is a simpler solution
@@ -297,7 +319,16 @@ struct AtomicList | |||
return MutBackwardIterator{ old_head }; | |||
} | |||
|
|||
// append the first head item if not already exists | |||
bool try_append_first_item( std::shared_ptr< ItemControlBlock > new_head ) |
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.
It is still possible to append two items with try_append_first_item()
, because it is still an ordinary append function just that it fails if two calls happen concurrently in such a way that one thread messes with the already read-out value of head
.
If two threads call try_append_first_item()
with such scheduling that the second thread initialized old_head
after the first thread already wrote to head
, the second thread will simply append another item after head.
it should be like
std::shared_ptr< ItemControlBlock > expected( nullptr );
std::shared_ptr< ItemControlBlock > const & desired = new_head;
std::atomic_compare_exchange_strong<ItemControlBlock>( &head, &expected, desired );
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.
yes this was my intention but wrote it during a meeting and than I forgot to add the empty element as old. I will update the branch with your suggestion.
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.
added your suggestion and forced push the change
On the first allocation in `chunk_list` the first chunk will be added only if needed. This avoids that memory is allocated if no allocation is required.
6ce9f56
to
d2a0d85
Compare
1287c5b
into
ComputationalRadiationPhysics:dev
On the first allocation in
chunk_list
the first chunk will be added only if needed.This prevents memory is allocated if no allocation is required.
This PR is addressing the comment: michaelsippel#1 (comment)