-
Notifications
You must be signed in to change notification settings - Fork 21
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
Use samllvec to reduce heap usage, use inline functions and general improvements #115
base: main
Are you sure you want to change the base?
Use samllvec to reduce heap usage, use inline functions and general improvements #115
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #115 +/- ##
==========================================
- Coverage 60.14% 60.13% -0.01%
==========================================
Files 33 33
Lines 16191 16236 +45
==========================================
+ Hits 9738 9764 +26
- Misses 6453 6472 +19 ☔ View full report in Codecov by Sentry. |
@@ -62,6 +62,7 @@ lazy_static = "1.4.0" | |||
thiserror = "1.0.47" | |||
futures = { version = "0.3.28" } | |||
async-trait = "0.1.82" | |||
smallvec = "1.13.2" |
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.
I assume you should update the lock file as well, or?
@@ -112,6 +114,10 @@ pub enum Field { | |||
ActuatorTarget, | |||
MetadataUnit, | |||
} | |||
#[derive(Debug, Clone)] | |||
pub struct StackVecField { | |||
pub svec: SmallVec<[Field; 3]>, |
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.
do we expect 3 to be the "optimal" number because if I understand correctly after 3 elements it will store it anyways in the heap right?
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.
for pub fields: StackVecField,
this might be the right approach since fields is [current, target, metadata]
@@ -1493,6 +1567,15 @@ impl AuthorizedAccess<'_, '_> { | |||
.cloned() | |||
} | |||
|
|||
pub async fn contains_id(&self, id: i32) -> bool { |
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.
same name as function in line 1263 intended?
@@ -849,6 +847,7 @@ async fn publish_values( | |||
} | |||
} | |||
|
|||
#[inline] |
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.
is this function not a bit too long for inline?
Improved performance by around 6%