| author | |
| committer | |
| log | d344bc604819cc97912c0f7deccaa3753f7d8c55 |
| tree | 0bd37b1938224d4f65d64d4d923de4f0ed3383e2 |
| parent | 4c06a93816860ce967019ff5bc0dd4b95ca90ccc |
| signature | Signed by SSH key SHA256:52mNGHRsVFBDED9IAX5pe+LRWUefqTbxEReunq21QvU (~clover) |
RasterImage::decode called imageops::resize, a generic function, so the
Triangle filter was compiled with draw at opt-level 0 instead of with image
at the opt-level 3 debug builds give dependencies. A 1024 px picture took
about 240 ms to shrink rather than 11 ms. The budget test shrinks twenty of
them one after another on the single picture thread, about five seconds on
an idle machine and past its twenty-second limit under load. Going through
DynamicImage::resize_exact runs the same filter as image compiled it.
Release builds were unaffected.
Under the same load, a_missing_section_file_reports_its_error_until_it_returns
failed for a separate reason. sync_status read the last sync time before it
took the observed error, so a step that finished in between showed its
cleared error next to the previous step's time. The time is now read under
the observed lock, after the error.
Fixes #149
Assisted-by: claude-opus-5.53 files changed, 11 insertions(+), 10 deletions(-)
crates/draw/src/render.rs+5-6| ... | @@ -137,12 +137,11 @@ impl RasterImage { | ... | @@ -137,12 +137,11 @@ impl RasterImage { |
| 137 | let size = [pixels.width(), pixels.height()]; | 137 | let size = [pixels.width(), pixels.height()]; |
| 138 | let shown = [0, 1].map(|axis| size[axis].min(within[axis]).max(1)); | 138 | let shown = [0, 1].map(|axis| size[axis].min(within[axis]).max(1)); |
| 139 | if shown != size { | 139 | if shown != size { |
| 140 | pixels = image::imageops::resize( | 140 | // Through `DynamicImage` the filter is compiled in `image`, optimised in debug builds |
| 141 | &pixels, | 141 | // too; `imageops::resize` called here would be compiled unoptimised with this crate. |
| 142 | shown[0], | 142 | pixels = image::DynamicImage::from(pixels) |
| 143 | shown[1], | 143 | .resize_exact(shown[0], shown[1], image::imageops::FilterType::Triangle) |
| 144 | image::imageops::FilterType::Triangle, | 144 | .into_rgba8(); |
| 145 | ); | ||
| 146 | } | 145 | } |
| 147 | Self::premultiplied(shown, pixels.into_raw()) | 146 | Self::premultiplied(shown, pixels.into_raw()) |
| 148 | } | 147 | } |
crates/notebook/src/background.rs+1-1| ... | @@ -495,7 +495,7 @@ impl Background { | ... | @@ -495,7 +495,7 @@ impl Background { |
| 495 | let replica = held.replica.upgrade()?; | 495 | let replica = held.replica.upgrade()?; |
| 496 | let observed = held.observed.upgrade()?; | 496 | let observed = held.observed.upgrade()?; |
| 497 | let synced = | 497 | let synced = |
| 498 | Some(worker.synced.load(Ordering::Acquire)).filter(|time| *time != 0); | 498 | || Some(worker.synced.load(Ordering::Acquire)).filter(|time| *time != 0); |
| 499 | Some(sync_status(&replica, synced, &observed)) | 499 | Some(sync_status(&replica, synced, &observed)) |
| 500 | }); | 500 | }); |
| 501 | let status = match live { | 501 | let status = match live { |
crates/notebook/src/session.rs+5-3| ... | @@ -2198,7 +2198,7 @@ pub(crate) type Observed = Option<Option<io::Error>>; | ... | @@ -2198,7 +2198,7 @@ pub(crate) type Observed = Option<Option<io::Error>>; |
| 2198 | 2198 | ||
| 2199 | pub(crate) fn sync_status( | 2199 | pub(crate) fn sync_status( |
| 2200 | replica: &Replica, | 2200 | replica: &Replica, |
| 2201 | synced: Option<u64>, | 2201 | synced: impl FnOnce() -> Option<u64>, |
| 2202 | observed: &Mutex<Observed>, | 2202 | observed: &Mutex<Observed>, |
| 2203 | ) -> Result<SyncStatus> { | 2203 | ) -> Result<SyncStatus> { |
| 2204 | let queued: i64 = crate::lock(&replica.section.connection)?.query_row( | 2204 | let queued: i64 = crate::lock(&replica.section.connection)?.query_row( |
| ... | @@ -2210,7 +2210,9 @@ pub(crate) fn sync_status( | ... | @@ -2210,7 +2210,9 @@ pub(crate) fn sync_status( |
| 2210 | .lock() | 2210 | .lock() |
| 2211 | .map_err(|_| io::Error::other("Synchronization observer panicked"))?; | 2211 | .map_err(|_| io::Error::other("Synchronization observer panicked"))?; |
| 2212 | Ok(SyncStatus { | 2212 | Ok(SyncStatus { |
| 2213 | synced, | 2213 | // Read after `observed`: the worker stores the time before it clears the error, so a |
| 2214 | // cleared error never shows beside the time of the step before it. | ||
| 2215 | synced: synced(), | ||
| 2214 | error: observed | 2216 | error: observed |
| 2215 | .as_ref() | 2217 | .as_ref() |
| 2216 | .and_then(Option::as_ref) | 2218 | .and_then(Option::as_ref) |
| ... | @@ -2732,7 +2734,7 @@ impl Section { | ... | @@ -2732,7 +2734,7 @@ impl Section { |
| 2732 | pub fn sync_status(&self) -> Result<SyncStatus> { | 2734 | pub fn sync_status(&self) -> Result<SyncStatus> { |
| 2733 | sync_status( | 2735 | sync_status( |
| 2734 | &self.replica, | 2736 | &self.replica, |
| 2735 | self.worker.as_ref().and_then(SyncWorker::synced), | 2737 | || self.worker.as_ref().and_then(SyncWorker::synced), |
| 2736 | &self.observed, | 2738 | &self.observed, |
| 2737 | ) | 2739 | ) |
| 2738 | } | 2740 | } |