Aj/dsd channel aggregator#801
Conversation
| @@ -134,7 +134,7 @@ impl Default for ContextBuffer { | |||
| /// Creates a new `ContextBuffer` with a default capacity of 5. | |||
There was a problem hiding this comment.
| /// Creates a new `ContextBuffer` with a default capacity of 5. | |
| /// Creates a new `ContextBuffer` with a default capacity of 5000. |
| .expect("lock poisoned") | ||
| .insert(metric) | ||
| { | ||
| if let Err(e) = self.aggr_handle.insert_batch(vec![metric]) { |
There was a problem hiding this comment.
Can't we add another method where its just an insert so we don't allocate a vector all the time?
There was a problem hiding this comment.
Yeah but a better option in this case will be to just insert them all in one call haha
There was a problem hiding this comment.
Oh but also the metric itself is heap allocated b/c it has strings and other nonconstant struct members so the vec wrapper is likely minimal actual overhead
duncanista
left a comment
duncanista
left a comment
There was a problem hiding this comment.
Everything looks good, I just don't like the vec[] allocation on every insert, wondering if we could just add another command for Insert w/o a batch, are even adding batches here at all?
| @@ -134,7 +134,7 @@ impl Default for ContextBuffer { | |||
| /// Creates a new `ContextBuffer` with a default capacity of 5. | |||
| /// | |||
| fn default() -> Self { | |||
There was a problem hiding this comment.
Could we add some comment to explain why bumping up to a much bigger value?
litianningdatadog
force-pushed
the
aj/dsd-channel-aggregator
branch
from
6211bac to
b743456
Compare
litianningdatadog
force-pushed
the
aj/dsd-channel-aggregator
branch
from
6079f34 to
fde9204
Compare
litianningdatadog
force-pushed
the
aj/dsd-channel-aggregator
branch
from
fde9204 to
b70af0c
Compare
3 participants
TODO update git sha when we merge DataDog/serverless-components#38