From 5abcfb99a673a310d93c158259107ece794058b8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adrian=20Wo=C5=BAniak?= Date: Fri, 15 Oct 2021 13:36:24 +0200 Subject: [PATCH] Reduce issues memory usage --- actors/websocket-actor/src/handlers/issues.rs | 39 ++-- shared/jirs-data/src/msg.rs | 11 +- web/src/model.rs | 28 ++- web/src/pages/project_page/model.rs | 11 +- web/src/pages/project_page/update.rs | 17 +- web/src/pages/project_settings_page/update.rs | 2 +- web/src/pages/project_settings_page/view.rs | 1 - web/src/ws/issue.rs | 182 +++++++++--------- web/src/ws/mod.rs | 34 ++-- 9 files changed, 176 insertions(+), 149 deletions(-) diff --git a/actors/websocket-actor/src/handlers/issues.rs b/actors/websocket-actor/src/handlers/issues.rs index f1d0b00d..a9c0e459 100644 --- a/actors/websocket-actor/src/handlers/issues.rs +++ b/actors/websocket-actor/src/handlers/issues.rs @@ -2,11 +2,8 @@ use std::collections::HashMap; use database_actor::issue_assignees::LoadAssignees; use database_actor::issues::{LoadProjectIssues, UpdateIssue}; -use jirs_data::msg::{WsMsgIssue, WsMsgProject}; -use jirs_data::{ - CreateIssuePayload, IssueAssignee, IssueFieldId, IssueId, IssueStatusId, ListPosition, - PayloadVariant, WsMsg, -}; +use jirs_data::msg::{IssueSync, WsMsgIssue, WsMsgProject}; +use jirs_data::{CreateIssuePayload, IssueAssignee, IssueFieldId, IssueId, PayloadVariant, WsMsg}; use crate::{db_or_debug_and_return, AsyncHandler, WebSocketActor, WsResult}; @@ -24,14 +21,12 @@ impl AsyncHandler for WebSocketActor { } WsMsgIssue::IssueCreate(payload) => self.exec(payload).await, WsMsgIssue::IssueDelete(id) => self.exec(DeleteIssue { id }).await, - WsMsgIssue::IssueSyncListPosition(sync) => { - self.exec(SyncIssueListPosition(sync)).await?; - Ok(None) - } + WsMsgIssue::IssueSyncListPosition(sync) => self.exec(SyncIssueListPosition(sync)).await, WsMsgIssue::IssueUpdated(_) => Ok(None), WsMsgIssue::IssueDeleted(_, _) => Ok(None), WsMsgIssue::IssueCreated(_) => Ok(None), + WsMsgIssue::IssueSyncedListPosition(_) => Ok(None), } } } @@ -196,27 +191,37 @@ impl AsyncHandler for WebSocketActor { } } -pub struct SyncIssueListPosition(pub Vec<(IssueId, ListPosition, IssueStatusId, Option)>); +pub struct SyncIssueListPosition(pub Vec); #[async_trait::async_trait] impl AsyncHandler for WebSocketActor { async fn exec(&mut self, msg: SyncIssueListPosition) -> WsResult { let _project_id = self.require_user_project()?.project_id; - for (issue_id, list_position, status_id, epic_id) in msg.0 { + let mut result = Vec::with_capacity(msg.0.len()); + for issue_sync in msg.0 { crate::actor_or_debug_and_ignore!( self, db, database_actor::issues::UpdateIssue { - issue_id, - list_position: Some(list_position), - issue_status_id: Some(status_id), - epic_id: Some(epic_id), + issue_id: issue_sync.id, + list_position: Some(issue_sync.list_position), + issue_status_id: Some(issue_sync.issue_status_id), + epic_id: Some(issue_sync.epic_id), ..Default::default() }, - |_| {}; async + |issue: database_actor::models::Issue| { + result.push(IssueSync { + id: issue.id, + list_position: issue.list_position, + issue_status_id: issue.issue_status_id, + epic_id: issue.epic_id, + }); + }; async ); } - self.exec(LoadIssues).await + Ok(Some(WsMsg::Issue(WsMsgIssue::IssueSyncedListPosition( + result, + )))) } } diff --git a/shared/jirs-data/src/msg.rs b/shared/jirs-data/src/msg.rs index 798e4ab5..d3494e3a 100644 --- a/shared/jirs-data/src/msg.rs +++ b/shared/jirs-data/src/msg.rs @@ -119,6 +119,14 @@ impl WsError { } } +#[derive(Serialize, Deserialize, Debug, Clone, PartialEq)] +pub struct IssueSync { + pub id: IssueId, + pub list_position: ListPosition, + pub issue_status_id: IssueStatusId, + pub epic_id: Option, +} + #[derive(Serialize, Deserialize, Debug, Clone, PartialEq)] pub enum WsMsgIssue { IssueUpdate(IssueId, IssueFieldId, PayloadVariant), @@ -127,7 +135,8 @@ pub enum WsMsgIssue { IssueDeleted(IssueId, NumberOfDeleted), IssueCreate(CreateIssuePayload), IssueCreated(Issue), - IssueSyncListPosition(Vec<(IssueId, ListPosition, IssueStatusId, Option)>), + IssueSyncListPosition(Vec), + IssueSyncedListPosition(Vec), } impl From for WsMsg { diff --git a/web/src/model.rs b/web/src/model.rs index ccb58023..55cf0e56 100644 --- a/web/src/model.rs +++ b/web/src/model.rs @@ -169,6 +169,12 @@ macro_rules! match_page { _ => return, } }; + ($model: ident, $ty: ident, $def: expr) => { + match &$model.page_content { + crate::model::PageContent::$ty(page) => page, + _ => return $def, + } + }; ($model: ident, $ty: ident; Empty) => { match &$model.page_content { crate::model::PageContent::$ty(page) => page, @@ -184,6 +190,12 @@ macro_rules! match_page_mut { _ => return, } }; + ($model: ident, $ty: ident, $def: expr) => { + match &mut $model.page_content { + PageContent::$ty(page) => page, + _ => return $def, + } + }; } #[derive(Debug)] @@ -288,7 +300,7 @@ impl Model { current_user_project: None, about_tooltip_visible: false, messages_tooltip_visible: false, - issues: vec![], + issue_ids: vec![], user_ids: vec![], users_by_id: HashMap::with_capacity(1_000), user_settings: None, @@ -354,15 +366,15 @@ impl Model { } pub fn epic_issue_ids(&self, epic_id: EpicId) -> Vec { - self.issues() + self.issue_ids .iter() - .filter_map(|issue| { - if issue.epic_id == Some(epic_id) { - Some(issue.id) - } else { - None - } + .filter(|id| { + self.issues_by_id + .get(id) + .filter(|issue| issue.epic_id == Some(epic_id)) + .is_some() }) + .copied() .collect() } diff --git a/web/src/pages/project_page/model.rs b/web/src/pages/project_page/model.rs index e4294706..00b360e3 100644 --- a/web/src/pages/project_page/model.rs +++ b/web/src/pages/project_page/model.rs @@ -39,6 +39,7 @@ impl ProjectPage { where IssueStream: std::iter::Iterator, { + let num_of_epics = epics.len(); let epics = vec![None].into_iter().chain( epics .iter() @@ -51,7 +52,7 @@ impl ProjectPage { && issue_filter_with_text(issue, page.text_filter.as_str()) && issue_filter_with_only_my(issue, page.only_my_filter, user) }); - let issues = if page.recently_updated_filter { + let mut issues = if page.recently_updated_filter { let mut m = HashMap::new(); let mut sorted: Vec<(IssueId, NaiveDateTime)> = issues .map(|issue| { @@ -71,12 +72,14 @@ impl ProjectPage { issues.collect() }; + issues.sort_by(|a, b| a.list_position.cmp(&b.list_position)); let issues_per_epic_id = { - let issues_len = issues.len(); issues .into_iter() - .fold(HashMap::with_capacity(issues_len), |mut m, issue| { - m.entry(issue.epic_id).or_insert_with(Vec::new).push(issue); + .fold(HashMap::with_capacity(num_of_epics), |mut m, issue| { + m.entry(issue.epic_id) + .or_insert_with(|| Vec::with_capacity(100)) + .push(issue); m }) }; diff --git a/web/src/pages/project_page/update.rs b/web/src/pages/project_page/update.rs index 1407ef46..f674dad7 100644 --- a/web/src/pages/project_page/update.rs +++ b/web/src/pages/project_page/update.rs @@ -5,6 +5,7 @@ use seed::prelude::Orders; use crate::components::styled_select::StyledSelectChanged; use crate::model::{Model, Page, PageContent}; use crate::pages::project_page::model::ProjectPage; +use crate::ws::issue::change_visible; use crate::ws::{board_load, send_ws_msg}; use crate::{ BoardPageChange, EditIssueModalSection, FieldId, Msg, OperationKind, PageChanged, ResourceKind, @@ -97,7 +98,9 @@ pub fn update(msg: Msg, model: &mut crate::model::Model, orders: &mut impl Order issue_bellow_id, ))) => crate::ws::issue::change_position(issue_bellow_id, model), Msg::PageChanged(PageChanged::Board(BoardPageChange::IssueDragOverStatus(status))) => { - crate::ws::issue::change_status(status, model) + if !crate::ws::issue::change_status(status, model) { + orders.skip(); + } } Msg::PageChanged(PageChanged::Board(BoardPageChange::IssueDropZone(_status))) => { crate::ws::issue::sync(model, orders) @@ -116,17 +119,7 @@ pub fn update(msg: Msg, model: &mut crate::model::Model, orders: &mut impl Order } } if rebuild_visible { - let visible_issues = ProjectPage::visible_issues( - crate::match_page!(model, Project), - model.epics(), - model.issue_statuses(), - model - .issue_ids - .iter() - .filter_map(|id| model.issues_by_id.get(id)), - model.user(), - ); - crate::match_page_mut!(model, Project).visible_issues = visible_issues; + change_visible(model); } } diff --git a/web/src/pages/project_settings_page/update.rs b/web/src/pages/project_settings_page/update.rs index f8f3951d..7b6434e1 100644 --- a/web/src/pages/project_settings_page/update.rs +++ b/web/src/pages/project_settings_page/update.rs @@ -118,7 +118,7 @@ pub fn update(msg: Msg, model: &mut Model, orders: &mut impl Orders) { Msg::PageChanged(PageChanged::ProjectSettings(ProjectPageChange::ColumnDropZone( _issue_status_id, ))) => { - sync(model, orders); + // sync(model, orders); } Msg::PageChanged(PageChanged::ProjectSettings(ProjectPageChange::EditIssueStatusName( id, diff --git a/web/src/pages/project_settings_page/view.rs b/web/src/pages/project_settings_page/view.rs index 65ae7355..6b861fde 100644 --- a/web/src/pages/project_settings_page/view.rs +++ b/web/src/pages/project_settings_page/view.rs @@ -209,7 +209,6 @@ fn columns_section(model: &Model, page: &ProjectSettingsPage) -> Node { .issue_ids .iter() .filter_map(|id| model.issues_by_id.get(id)) - .iter() .fold( HashMap::with_capacity(model.issue_statuses_by_id.len()), |mut h, issue| { diff --git a/web/src/ws/issue.rs b/web/src/ws/issue.rs index d8e04245..9bf97638 100644 --- a/web/src/ws/issue.rs +++ b/web/src/ws/issue.rs @@ -1,4 +1,4 @@ -use jirs_data::msg::WsMsgIssue; +use jirs_data::msg::{IssueSync, WsMsgIssue}; use jirs_data::*; use seed::prelude::Orders; use seed::*; @@ -30,59 +30,51 @@ pub fn change_position(below_id: EpicId, model: &mut Model) { } let (issue_status_id, epic_id) = model + .issues_by_id + .get(&dragged_id) + .map(|issue| (issue.issue_status_id, issue.epic_id)) + .unwrap_or_default(); + + let mut issues = model .issue_ids .iter() - .filter_map(|id| model.issues_by_id.get(id)) - .find_map(|issue| { - if issue.id == dragged_id { - Some((issue.issue_status_id, issue.epic_id)) - } else { - None - } + .filter_map(|id| { + model + .issues_by_id + .get(id) + .filter(|issue| { + issue.issue_status_id == issue_status_id && issue.epic_id == epic_id + }) + .map(|issue| (issue.id, issue.list_position)) }) - .unwrap_or_default(); + .collect::>(); + issues.sort_by(|(_, a), (_, b)| a.cmp(b)); - let mut issues: Vec = model - .issues_mut() - .drain_filter(|issue| issue.issue_status_id == issue_status_id && issue.epic_id == epic_id) - .collect(); - issues.sort_by(|a, b| a.list_position.cmp(&b.list_position)); - - let below_idx = issues - .iter() - .position(|issue| issue.id == below_id) - .unwrap_or_default(); - let dragged_idx = issues - .iter() - .position(|issue| issue.id == dragged_id) - .unwrap_or_default(); - - let dragged = issues.remove(dragged_idx); - issues.insert(below_idx, dragged); - - let mut changed = Vec::with_capacity(issues.len()); - for (idx, mut issue) in issues.into_iter().enumerate() { - issue.list_position = idx as i32; - if let Some(iss) = model.issues_by_id.get_mut(&issue.id) { - iss.list_position = issue.list_position; - } - changed.push((issue.id, issue.list_position)); - model.issues_mut().push(issue); + { + let below_idx = issues + .iter() + .position(|(id, _)| *id == below_id) + .unwrap_or_default(); + let dragged_idx = issues + .iter() + .position(|(id, _)| *id == dragged_id) + .unwrap_or_default(); + let dragged = issues.remove(dragged_idx); + issues.insert(below_idx, dragged); } - let visible = ProjectPage::visible_issues( - crate::match_page!(model, Project), - model.epics(), - model.issue_statuses(), - model.issues(), - model.user(), - ); - if let PageContent::Project(project_page) = &mut model.page_content { - project_page.visible_issues = visible; - for (id, _) in changed.iter() { - project_page.issue_drag.mark_dirty(*id); + { + for (idx, (id, _)) in issues.into_iter().enumerate() { + if let Some(issue) = model.issues_by_id.get_mut(&id) { + issue.list_position = idx as i32; + } + crate::match_page_mut!(model, Project) + .issue_drag + .mark_dirty(id); } } + + change_visible(model); } pub fn sync(model: &mut Model, orders: &mut impl Orders) { @@ -91,19 +83,17 @@ pub fn sync(model: &mut Model, orders: &mut impl Orders) { _ => return, }; - let changes: Vec<(EpicId, ListPosition, IssueStatusId, Option)> = dirty + let changes = dirty .into_iter() .filter_map(|id| { - model.issues_by_id.get(&id).map(|issue| { - ( - issue.id, - issue.list_position, - issue.issue_status_id, - issue.epic_id, - ) + model.issues_by_id.get(&id).map(|issue| IssueSync { + id: issue.id, + list_position: issue.list_position, + issue_status_id: issue.issue_status_id, + epic_id: issue.epic_id, }) }) - .collect(); + .collect::>(); send_ws_msg( WsMsg::Issue(WsMsgIssue::IssueSyncListPosition(changes)), @@ -113,15 +103,18 @@ pub fn sync(model: &mut Model, orders: &mut impl Orders) { crate::match_page_mut!(model, Project).issue_drag.clear(); } -pub fn change_status(status_id: IssueStatusId, model: &mut Model) { - let dragged_id = match crate::match_page!(model, Project) +pub fn change_status(status_id: IssueStatusId, model: &mut Model) -> bool { + let dragged_id = match crate::match_page!(model, Project, false) .issue_drag .dragged_id .as_ref() .cloned() { Some(issue_id) => issue_id, - _ => return error!("Nothing is dragged"), + _ => { + error!("Nothing is dragged"); + return false; + } }; let (issue_status_id, epic_id) = model .issues_by_id @@ -129,45 +122,50 @@ pub fn change_status(status_id: IssueStatusId, model: &mut Model) { .map(|issue| (issue.issue_status_id, issue.epic_id)) .unwrap_or_default(); if status_id == issue_status_id { - return; + return false; } - let mut issues = { - let mut h = std::mem::take(&mut model.issues_by_id); - h.keys() - .filter_map(|id| h.remove(id)) - .collect::>() - } - .drain_filter(|issue| { - if issue.id == dragged_id { - issue.issue_status_id = status_id; - } - issue.issue_status_id == status_id && issue.epic_id == epic_id - }) - .collect::>(); - - issues.sort_by(|a, b| a.list_position.cmp(&b.list_position)); - - let mut dirty = vec![]; - for mut issue in issues { - if issue.id == dragged_id { - issue.issue_status_id = status_id; - if let Some(iss) = model.issues_by_id.get_mut(&issue.id) { - iss.issue_status_id = status_id; - } - } - - dirty.push(issue.id); - model.issue_ids.push(issue.id); - model.issues_by_id.insert(issue.id, issue); - } + let epic_and_status_issues_count = model + .issue_ids + .iter() + .filter_map(|id| { + model + .issues_by_id + .get(id) + .filter(|issue| same_epic_and_status(status_id, epic_id, issue)) + }) + .count(); { - let project_page = crate::match_page_mut!(model, Project); - for id in dirty { + if let Some(issue) = model.issues_by_id.get_mut(&dragged_id) { + issue.issue_status_id = status_id; + issue.list_position = epic_and_status_issues_count as ListPosition; + } + } + + let issues_in_column = model + .issue_ids + .iter() + .filter_map(|id| { + model + .issues_by_id + .get(id) + .filter(|issue| same_epic_and_status(status_id, epic_id, issue)) + .map(|issue| issue.id) + }) + .collect::>(); + + { + let project_page = crate::match_page_mut!(model, Project, false); + for id in issues_in_column { project_page.issue_drag.mark_dirty(id); } } + change_visible(model); + true +} + +pub fn change_visible(model: &mut Model) { let visible = ProjectPage::visible_issues( crate::match_page!(model, Project), model.epics(), @@ -181,3 +179,7 @@ pub fn change_status(status_id: IssueStatusId, model: &mut Model) { crate::match_page_mut!(model, Project).visible_issues = visible; } + +fn same_epic_and_status(status_id: IssueStatusId, epic_id: Option, issue: &&Issue) -> bool { + issue.issue_status_id == status_id && issue.epic_id == epic_id +} diff --git a/web/src/ws/mod.rs b/web/src/ws/mod.rs index 6c4972ea..c8aedaf5 100644 --- a/web/src/ws/mod.rs +++ b/web/src/ws/mod.rs @@ -253,21 +253,16 @@ pub fn update(msg: &mut WsMsg, model: &mut Model, orders: &mut impl Orders) // issues WsMsg::Project(WsMsgProject::ProjectIssuesLoaded(v)) => { v.sort_by(|a, b| (a.list_position as i64).cmp(&(b.list_position as i64))); - { - model.issue_ids = Vec::with_capacity(v.len()); - model.issues_by_id = - std::mem::take(v) - .into_iter() - .fold(HashMap::with_capacity(v.len()), |h, o| { - model.issue_ids.push(o.id); - h.insert(o.id, o); - h - }); - }; - model.issues_by_id.clear(); - for issue in v { - model.issues_by_id.insert(issue.id, issue.clone()); - } + let len = v.len(); + model.issue_ids = Vec::with_capacity(len); + model.issues_by_id = + std::mem::take(v) + .into_iter() + .fold(HashMap::with_capacity(len), |mut h, o| { + model.issue_ids.push(o.id); + h.insert(o.id, o); + h + }); orders.send_msg(Msg::ResourceChanged( ResourceKind::Issue, @@ -312,6 +307,15 @@ pub fn update(msg: &mut WsMsg, model: &mut Model, orders: &mut impl Orders) Some(*id), )); } + WsMsg::Issue(WsMsgIssue::IssueSyncedListPosition(sync)) => { + for o in sync { + if let Some(issue) = model.issues_by_id.get_mut(&o.id) { + issue.list_position = o.list_position; + issue.issue_status_id = o.issue_status_id; + issue.epic_id = o.epic_id; + } + } + } // users WsMsg::Project(WsMsgProject::ProjectUsersLoaded(v)) => { model.user_ids = v.iter().map(|u| u.id).collect();