Skip to content

Conversation

@gardnervickers
Copy link
Contributor

This reduces the size at construction of the batchMetadata queue from 16
entries to 5. The backing array will be sized to 8 elements, cutting the array size
in half.

We know that we will only allow a max of 5 entries in this queue, so this change will not
change the alloc/copy behavior.

Copy link
Member

@satishd satishd left a comment

Choose a reason for hiding this comment

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

thanks @gardnervickers for the PR, left a minor comment.

Copy link
Member

Choose a reason for hiding this comment

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

minor: you may want to have it as new mutable.Queue[BatchMetadata](NumBatchesToRetain) instead of harcoding directly.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, good suggestion :)

Copy link
Member

@satishd satishd left a comment

Choose a reason for hiding this comment

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

thanks @gardnervickers for addressing the review comments. LGTM.

This reduces the size at construction of the batchMetadata queue from 16
entries to 5. The backing array will resize to 8 elements, cutting the queue size
in half.

We know that we will only allow a max of 5 entries in this queue, so this change will not
change the alloc/copy behavior.
@gardnervickers gardnervickers force-pushed the size-batch-metadata-queue branch from 17898ea to ba6958a Compare May 12, 2021 21:46
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