-
Notifications
You must be signed in to change notification settings - Fork 167
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
Perf/reduce mem allocations #1346
Conversation
0331128
to
cdc4b6b
Compare
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.
A couple of small nitpicks and questions, but otherwise this looks like a really nice improvement so far.
let mut element_render_states = RenderElementStates { | ||
states: HashMap::with_capacity(elements.len()), | ||
}; | ||
|
||
let mut damage = std::mem::take(&mut self.damage); | ||
let mut element_damage = std::mem::take(&mut self.element_damage); |
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 cause new Vec's to be created, no? Why are we not directly using self.damage
here, we just cleared it anyway?
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 returned here:
smithay/src/backend/renderer/damage/mod.rs
Line 681 in 13bc424
std::mem::swap(&mut self.damage, &mut damage); |
self.damage
, yeah.
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.
Right and empty Vec does nothing, but it is still confusing to read. If there is no reason to do it this way, I would prefer to just use the value directly in all these cases.
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.
mostly done, we need to take element_damage
as we move it around a bit
da48922
to
b418255
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1346 +/- ##
==========================================
- Coverage 20.61% 20.50% -0.11%
==========================================
Files 161 161
Lines 26145 25987 -158
==========================================
- Hits 5390 5329 -61
+ Misses 20755 20658 -97
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
b418255
to
7911716
Compare
7911716
to
9742b12
Compare
9742b12
to
f4d5874
Compare
attempt on reducing mem allocations in hot-code paths and some performance improvements
not sure how to best handle the
Space::refresh
/Output::enter
/Output::leave
stuff. this is called way to oftenimo and we need to get the alive checks out of enter/leave.