From b2800ac267a76c19ce3229b031d809c4539ba5e7 Mon Sep 17 00:00:00 2001 From: licunlong Date: Wed, 31 May 2023 11:10:42 +0800 Subject: [PATCH 4/9] fix: move RefCell to the inside of Table, no functional change --- core/bin/unit/data/dep_conf.rs | 2 +- core/bin/unit/data/state.rs | 2 +- core/bin/unit/data/table.rs | 49 +++++++++------------- core/bin/unit/datastore/sets.rs | 2 +- core/bin/unit/entry/ratelimit.rs | 2 +- core/bin/utils/table.rs | 71 +++++++++++++++++--------------- 6 files changed, 60 insertions(+), 68 deletions(-) diff --git a/core/bin/unit/data/dep_conf.rs b/core/bin/unit/data/dep_conf.rs index 9c203c5..e61ce5b 100644 --- a/core/bin/unit/data/dep_conf.rs +++ b/core/bin/unit/data/dep_conf.rs @@ -13,7 +13,7 @@ use std::collections::HashMap; use sysmaster::unit::UnitRelations; -#[derive(Default)] +#[derive(Default, Clone)] pub struct UnitDepConf { pub deps: HashMap>, } diff --git a/core/bin/unit/data/state.rs b/core/bin/unit/data/state.rs index fcdd51e..68e8607 100644 --- a/core/bin/unit/data/state.rs +++ b/core/bin/unit/data/state.rs @@ -12,7 +12,7 @@ use sysmaster::unit::{UnitActiveState, UnitNotifyFlags}; -#[derive(Debug)] +#[derive(Debug, Clone)] pub(crate) struct UnitState { pub(crate) os: UnitActiveState, pub(crate) ns: UnitActiveState, diff --git a/core/bin/unit/data/table.rs b/core/bin/unit/data/table.rs index 480a026..161ac5f 100644 --- a/core/bin/unit/data/table.rs +++ b/core/bin/unit/data/table.rs @@ -15,17 +15,16 @@ use super::state::UnitState; use crate::job::JobResult; use crate::unit::entry::StartLimitResult; use crate::utils::table::{Table, TableSubscribe}; -use std::cell::RefCell; use std::rc::Rc; use sysmaster::rel::ReStation; #[allow(clippy::type_complexity)] pub struct DataManager { tables: ( - RefCell>, // [0]unit-dep-config - RefCell>, // [1]unit-state - RefCell>, // [2]unit-start-limit-hit - RefCell>, // [3] unit-job-timeout + Table, // [0]unit-dep-config + Table, // [1]unit-state + Table, // [2]unit-start-limit-hit + Table, // [3] unit-job-timeout ), } @@ -35,8 +34,8 @@ impl ReStation for DataManager { // reload fn entry_clear(&self) { - self.tables.0.borrow_mut().data_clear(); - self.tables.1.borrow_mut().data_clear(); + self.tables.0.data_clear(); + self.tables.1.data_clear(); } } @@ -51,12 +50,7 @@ impl Drop for DataManager { impl DataManager { pub fn new() -> DataManager { DataManager { - tables: ( - RefCell::new(Table::new()), - RefCell::new(Table::new()), - RefCell::new(Table::new()), - RefCell::new(Table::new()), - ), + tables: (Table::new(), Table::new(), Table::new(), Table::new()), } } @@ -66,7 +60,7 @@ impl DataManager { ud_config: UnitDepConf, ) -> Option { { - let old = self.tables.0.borrow_mut().insert(u_name, ud_config); + let old = self.tables.0.insert(u_name, ud_config); old } } @@ -76,8 +70,7 @@ impl DataManager { name: &str, subscriber: Rc>, ) -> Option>> { - let mut table = self.tables.0.borrow_mut(); - table.subscribe(name.to_string(), subscriber) + self.tables.0.subscribe(name.to_string(), subscriber) } pub(crate) fn insert_unit_state( @@ -85,8 +78,7 @@ impl DataManager { u_name: String, u_state: UnitState, ) -> Option { - let mut table = self.tables.1.borrow_mut(); - table.insert(u_name, u_state) + self.tables.1.insert(u_name, u_state) } pub(crate) fn register_unit_state( @@ -94,8 +86,7 @@ impl DataManager { name: &str, subscriber: Rc>, ) -> Option>> { - let mut table = self.tables.1.borrow_mut(); - table.subscribe(name.to_string(), subscriber) + self.tables.1.subscribe(name.to_string(), subscriber) } pub(crate) fn insert_start_limit_result( @@ -103,8 +94,7 @@ impl DataManager { u_name: String, start_limit_res: StartLimitResult, ) -> Option { - let mut table = self.tables.2.borrow_mut(); - table.insert(u_name, start_limit_res) + self.tables.2.insert(u_name, start_limit_res) } pub(crate) fn register_start_limit_result( @@ -112,8 +102,7 @@ impl DataManager { name: &str, subscriber: Rc>, ) -> Option>> { - let mut table = self.tables.2.borrow_mut(); - table.subscribe(name.to_string(), subscriber) + self.tables.2.subscribe(name.to_string(), subscriber) } pub(crate) fn insert_job_result( @@ -121,8 +110,7 @@ impl DataManager { u_name: String, job_result: JobResult, ) -> Option { - let mut table = self.tables.3.borrow_mut(); - table.insert(u_name, job_result) + self.tables.3.insert(u_name, job_result) } pub(crate) fn register_job_result( @@ -130,19 +118,20 @@ impl DataManager { name: &str, subscriber: Rc>, ) -> Option>> { - let mut table = self.tables.3.borrow_mut(); - table.subscribe(name.to_string(), subscriber) + self.tables.3.subscribe(name.to_string(), subscriber) } // repeating protection pub(crate) fn clear(&self) { - self.tables.0.borrow_mut().clear(); - self.tables.1.borrow_mut().clear(); + self.tables.0.clear(); + self.tables.1.clear(); } } #[cfg(test)] mod tests { + use std::cell::RefCell; + use super::*; use crate::unit::UnitRelations; use crate::utils::table::TableOp; diff --git a/core/bin/unit/datastore/sets.rs b/core/bin/unit/datastore/sets.rs index 677d21b..538df8f 100644 --- a/core/bin/unit/datastore/sets.rs +++ b/core/bin/unit/datastore/sets.rs @@ -54,7 +54,7 @@ impl UnitSets { } pub(super) fn get(&self, name: &str) -> Option> { - self.t.borrow().get(&name.to_string()).cloned() + self.t.borrow().get(&name.to_string()) } pub(super) fn get_all(&self) -> Vec> { diff --git a/core/bin/unit/entry/ratelimit.rs b/core/bin/unit/entry/ratelimit.rs index 2f2a446..2c67232 100644 --- a/core/bin/unit/entry/ratelimit.rs +++ b/core/bin/unit/entry/ratelimit.rs @@ -17,7 +17,7 @@ pub(super) struct StartLimit { start_limit: RefCell, } -#[derive(PartialEq, Eq)] +#[derive(PartialEq, Eq, Clone)] pub(crate) enum StartLimitResult { StartLimitNotHit, StartLimitHit, diff --git a/core/bin/utils/table.rs b/core/bin/utils/table.rs index 90846cc..e1768d2 100644 --- a/core/bin/utils/table.rs +++ b/core/bin/utils/table.rs @@ -10,6 +10,7 @@ // NON-INFRINGEMENT, MERCHANTABILITY OR FIT FOR A PARTICULAR PURPOSE. // See the Mulan PSL v2 for more details. +use std::cell::RefCell; use std::collections::HashMap; use std::hash::Hash; use std::rc::Rc; @@ -30,47 +31,45 @@ pub trait TableSubscribe { //#[derive(Debug)] pub struct Table { - data: HashMap, // key + value - subscribers: HashMap>>, // key: name, value: subscriber + data: RefCell>, // key + value + subscribers: RefCell>>>, // key: name, value: subscriber } impl Table where K: Eq + Hash + Clone, + V: Clone, { pub fn new() -> Table { Table { - data: HashMap::new(), - subscribers: HashMap::new(), + data: RefCell::new(HashMap::new()), + subscribers: RefCell::new(HashMap::new()), } } - pub fn data_clear(&mut self) { + pub fn data_clear(&self) { // clear all data without notifying subscribers - self.data.clear(); + self.data.borrow_mut().clear(); } - pub fn clear(&mut self) { + pub fn clear(&self) { // clear all, including data and subscribers self.data_clear(); - self.subscribers.clear(); + self.subscribers.borrow_mut().clear(); } - pub fn insert(&mut self, k: K, v: V) -> Option { + pub fn insert(&self, k: K, v: V) -> Option { let key = k.clone(); - let ret = self.data.insert(k, v); - let value = self - .data - .get(&key) - .expect("something inserted is not found."); - let op = TableOp::TableInsert(&key, value); + let ret = self.data.borrow_mut().insert(k, v); + let value = self.get(&key).expect("something inserted is not found."); + let op = TableOp::TableInsert(&key, &value); self.notify(&op); ret } #[allow(dead_code)] - pub fn remove(&mut self, k: &K) -> Option { - let ret = self.data.remove(k); + pub fn remove(&self, k: &K) -> Option { + let ret = self.data.borrow_mut().remove(k); if let Some(v) = &ret { let op = TableOp::TableRemove(k, v); self.notify(&op); @@ -78,29 +77,33 @@ where ret } - pub fn get(&self, k: &K) -> Option<&V> { - self.data.get(k) + pub fn get(&self, k: &K) -> Option { + self.data.borrow().get(k).cloned() } - pub fn get_all(&self) -> Vec<&V> { - self.data.values().collect::>() + pub fn get_all(&self) -> Vec { + self.data + .borrow() + .values() + .map(|v| v.clone()) + .collect::>() } pub fn subscribe( - &mut self, + &self, name: String, subscriber: Rc>, ) -> Option>> { - self.subscribers.insert(name, subscriber) + self.subscribers.borrow_mut().insert(name, subscriber) } #[allow(dead_code)] - pub fn unsubscribe(&mut self, name: &str) -> Option>> { - self.subscribers.remove(name) + pub fn unsubscribe(&self, name: &str) -> Option>> { + self.subscribers.borrow_mut().remove(name) } fn notify(&self, op: &TableOp<'_, K, V>) { - for (_, subscriber) in self.subscribers.iter() { + for (_, subscriber) in self.subscribers.borrow().iter() { if subscriber.filter(op) { subscriber.notify(op); } @@ -115,7 +118,7 @@ mod tests { #[test] fn table_insert() { - let mut table: Table = Table::new(); + let table: Table = Table::new(); let old = table.insert(1, 'a'); assert_eq!(old, None); @@ -129,7 +132,7 @@ mod tests { #[test] fn table_remove() { - let mut table: Table = Table::new(); + let table: Table = Table::new(); let old = table.remove(&1); assert_eq!(old, None); @@ -148,25 +151,25 @@ mod tests { #[test] fn table_get() { - let mut table: Table = Table::new(); + let table: Table = Table::new(); let value = table.get(&1); assert_eq!(value, None); table.insert(1, 'a'); let value = table.get(&1); - assert_eq!(value.cloned(), Some('a')); + assert_eq!(value, Some('a')); let value = table.get(&2); assert_eq!(value, None); table.insert(2, 'b'); let value = table.get(&2); - assert_eq!(value.cloned(), Some('b')); + assert_eq!(value, Some('b')); } #[test] fn table_subscribe() { - let mut table: Table = Table::new(); + let table: Table = Table::new(); let sub_test1 = Rc::new(TableTest::new()); let sub_test2 = Rc::new(TableTest::new()); @@ -185,7 +188,7 @@ mod tests { #[test] fn table_unsubscribe() { - let mut table: Table = Table::new(); + let table: Table = Table::new(); let sub_test1 = Rc::new(TableTest::new()); let sub_test2 = Rc::new(TableTest::new()); @@ -209,7 +212,7 @@ mod tests { #[test] fn table_notify() { - let mut table: Table = Table::new(); + let table: Table = Table::new(); let sub_test1 = Rc::new(TableTest::new()); let sub_test2 = Rc::new(TableTest::new()); -- 2.30.2