diff --git a/Cargo.toml b/Cargo.toml index bfa471840968..28a32e59067f 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -106,7 +106,7 @@ futures = { version = "0.3" } itertools = { version = "0.10.5" } lazy_static = { version = "1.4" } paste = { version = "1.0" } -serde_json = { version = "1.0" } +serde_json = { version = "1.0", features = ["raw_value"] } smallvec = { version = "1.0.0" } weak-table = { version = "0.3.0" } gen-iter = { version = "0.2.1" } diff --git a/app/gui/controller/engine-protocol/src/language_server/types.rs b/app/gui/controller/engine-protocol/src/language_server/types.rs index ed6c3ccb6d59..f349974e0230 100644 --- a/app/gui/controller/engine-protocol/src/language_server/types.rs +++ b/app/gui/controller/engine-protocol/src/language_server/types.rs @@ -108,7 +108,8 @@ impl Path { // ==================== /// Notification generated by the Language Server. -#[derive(Clone, Debug, Deserialize, PartialEq, Serialize, IntoStaticStr)] +#[derive(Clone, Debug, Deserialize, IntoStaticStr, Derivative)] +#[derivative(PartialEq)] #[serde(tag = "method", content = "params")] pub enum Notification { /// Filesystem event occurred for a watched path. @@ -128,7 +129,7 @@ pub enum Notification { /// Sent from the server to the client to inform about new information for certain expressions /// becoming available. This notification is superseded by executionContext/expressionUpdates. #[serde(rename = "executionContext/expressionValuesComputed")] - ExpressionValuesComputed(serde_json::Value), + ExpressionValuesComputed(#[derivative(PartialEq = "ignore")] serde::de::IgnoredAny), /// Sent from the server to the client to inform about new information for certain expressions /// becoming available. diff --git a/app/gui/src/controller/graph/widget/configuration.rs b/app/gui/src/controller/graph/widget/configuration.rs index af3d0010d436..d424591d6637 100644 --- a/app/gui/src/controller/graph/widget/configuration.rs +++ b/app/gui/src/controller/graph/widget/configuration.rs @@ -22,6 +22,7 @@ use ide_view::graph_editor::ArgumentWidgetConfig; /// data. Allows for partial deserialization: if any of the widget definitions fails to deserialize, /// it will be skipped, but the deserialization will continue. All errors are returned as a separate /// list. +#[profile(Debug)] pub fn deserialize_widget_definitions( data: &VisualizationUpdateData, db: &SuggestionDatabase, diff --git a/app/gui/src/presenter/graph/visualization.rs b/app/gui/src/presenter/graph/visualization.rs index de530ecc06c8..66fec7642c47 100644 --- a/app/gui/src/presenter/graph/visualization.rs +++ b/app/gui/src/presenter/graph/visualization.rs @@ -22,28 +22,32 @@ use ide_view::graph_editor::component::visualization as visualization_view; // === Model === // ============= -#[derive(Clone, CloneRef, Debug)] +#[derive(Debug)] struct Model { controller: controller::Visualization, graph_view: view::graph_editor::GraphEditor, manager: Rc, error_manager: Rc, state: Rc, + shown: RefCell>, } impl Model { /// Handle the showing visualization UI. fn visualization_shown(&self, node_id: ViewNodeId, metadata: visualization_view::Metadata) { + self.shown.borrow_mut().insert(node_id); self.update_visualization(node_id, &self.manager, Some(metadata)); } /// Handle the hiding in UI. fn visualization_hidden(&self, node_id: view::graph_editor::NodeId) { + self.shown.borrow_mut().remove(&node_id); self.update_visualization(node_id, &self.manager, None); } /// Handle the node removal in UI. fn node_removed(&self, node_id: view::graph_editor::NodeId) { + self.shown.borrow_mut().remove(&node_id); if self.state.ast_node_id_of_view(node_id).is_some() { self.update_visualization(node_id, &self.manager, None); self.update_visualization(node_id, &self.error_manager, None); @@ -56,8 +60,10 @@ impl Model { node_id: ViewNodeId, preprocessor: visualization_view::instance::PreprocessorConfiguration, ) { - let metadata = visualization_view::Metadata { preprocessor }; - self.update_visualization(node_id, &self.manager, Some(metadata)) + if self.shown.borrow().contains(&node_id) { + let metadata = visualization_view::Metadata { preprocessor }; + self.update_visualization(node_id, &self.manager, Some(metadata)) + } } /// Handle the error change on given node: attach/detach the error visualization if needed. @@ -95,7 +101,7 @@ impl Model { data: VisualizationUpdateData, ) { if let Some(view_id) = self.state.view_id_of_ast_node(target) { - match deserialize_visualization_data(data) { + match visualization_view::Data::json(&data) { Ok(data) => update_endpoint.emit((view_id, data)), Err(err) => { // TODO [mwu]: We should consider having the visualization also accept error @@ -178,6 +184,7 @@ impl Visualization { manager: manager.clone_ref(), error_manager: error_manager.clone_ref(), state, + shown: default(), }); frp::extend! { network @@ -296,18 +303,3 @@ impl Visualization { self } } - - - -// ======================== -// === Helper Functions === -// ======================== - -fn deserialize_visualization_data( - data: VisualizationUpdateData, -) -> FallibleResult { - let binary = data.as_ref(); - let as_text = std::str::from_utf8(binary)?; - let as_json: serde_json::Value = serde_json::from_str(as_text)?; - Ok(visualization_view::Data::from(as_json)) -} diff --git a/app/gui/view/examples/visualization/src/lib.rs b/app/gui/view/examples/visualization/src/lib.rs index 1462baaa2cce..3a9374afebad 100644 --- a/app/gui/view/examples/visualization/src/lib.rs +++ b/app/gui/view/examples/visualization/src/lib.rs @@ -138,8 +138,8 @@ fn init(app: &Application) { let data = generate_data( (time_info.since_animation_loop_started.unchecked_raw() / 1000.0).into(), ); - let content = serde_json::to_value(data).unwrap(); - let data = Data::from(content); + let content = serde_json::to_string(&data).unwrap(); + let data = Data::json(content.as_bytes()).unwrap(); visualization.send_data.emit(data); diff --git a/app/gui/view/graph-editor/src/builtin/visualization/native/bubble_chart.rs b/app/gui/view/graph-editor/src/builtin/visualization/native/bubble_chart.rs index 6cfa7305a084..3401999c028a 100644 --- a/app/gui/view/graph-editor/src/builtin/visualization/native/bubble_chart.rs +++ b/app/gui/view/graph-editor/src/builtin/visualization/native/bubble_chart.rs @@ -51,19 +51,10 @@ pub struct BubbleChartModel { } impl BubbleChartModel { + #[profile(Debug)] #[allow(clippy::question_mark)] fn receive_data(&self, data: &Data) -> Result<(), DataError> { - let data_inner = match data { - Data::Json { content } => content, - _ => return Err(DataError::BinaryNotSupported), - }; - let data_inner: &serde_json::Value = data_inner; - let data_inner: Rc>> = - if let Ok(result) = serde_json::from_value(data_inner.clone()) { - result - } else { - return Err(DataError::InvalidJsonText); - }; + let data_inner: Rc>> = data.as_json()?.deserialize()?; // Avoid re-creating views, if we have already created some before. let mut views = self.views.borrow_mut(); diff --git a/app/gui/view/graph-editor/src/builtin/visualization/native/error.rs b/app/gui/view/graph-editor/src/builtin/visualization/native/error.rs index 80faec4f64cc..7e2836857c9b 100644 --- a/app/gui/view/graph-editor/src/builtin/visualization/native/error.rs +++ b/app/gui/view/graph-editor/src/builtin/visualization/native/error.rs @@ -139,7 +139,7 @@ impl Error { /// Sets the visualization data directly from the [`Input`] structure (not from the serialized /// JSON). - pub fn set_data(&self, input: &Input) { + pub fn set_data(&self, input: Input) { self.model.set_data(input); } @@ -207,24 +207,19 @@ impl Model { self.reload_style(); } + #[profile(Debug)] fn receive_data(&self, data: &Data) -> Result<(), DataError> { - match data { - Data::Json { content } => { - let input_result = serde_json::from_value(content.deref().clone()); - let input: Input = input_result.map_err(|_| DataError::InvalidDataType)?; - self.set_data(&input); - Ok(()) - } - Data::Binary => Err(DataError::BinaryNotSupported), - } + let input: Input = data.as_json()?.deserialize()?; + self.set_data(input); + Ok(()) } - fn set_data(&self, input: &Input) { + fn set_data(&self, input: Input) { if let Some(kind) = input.kind { - self.messages.insert(kind, input.message.clone().into()); if kind == self.displayed.get() { self.dom.dom().set_inner_text(&input.message); } + self.messages.insert(kind, input.message.into()); } // else we don't update the text, as the node does not contain error anymore. The // visualization will be hidden once we receive expression update message. diff --git a/app/gui/view/graph-editor/src/builtin/visualization/native/text_visualization/text_provider.rs b/app/gui/view/graph-editor/src/builtin/visualization/native/text_visualization/text_provider.rs index 0c6932908f57..8ec51bd0496d 100644 --- a/app/gui/view/graph-editor/src/builtin/visualization/native/text_visualization/text_provider.rs +++ b/app/gui/view/graph-editor/src/builtin/visualization/native/text_visualization/text_provider.rs @@ -269,30 +269,19 @@ struct LazyGridDataUpdate { impl TryFrom for LazyGridDataUpdate { type Error = visualization::DataError; + #[profile(Debug)] fn try_from(data: visualization::Data) -> Result { - if let visualization::Data::Json { content } = data { - let grid_data = serde_json::from_value(content.deref().clone()); - let (data, update_type) = match grid_data { - Ok(data) => (data, UpdateType::PartialUpdate), - Err(_) => { - let data_str = if content.is_string() { - // We need to access the content `as_str` to preserve newlines. Just using - // `content.to_string()` would turn them into the characters `\n` in the - // output. The unwrap can never fail, as we just - // checked that the content is a string. - Ok(content.as_str().map(|s| s.to_owned()).unwrap_or_default()) - } else { - serde_json::to_string_pretty(&*content) - }; - let data_str = - data_str.unwrap_or_else(|e| format!("")); - (data_str.into(), UpdateType::FullUpdate) - } - }; - Ok(LazyGridDataUpdate { data, update_type }) - } else { - Err(visualization::DataError::BinaryNotSupported) - } + let content = data.as_json()?; + let grid_data: Result = content.deserialize(); + let (data, update_type) = match grid_data { + Ok(data) => (data, UpdateType::PartialUpdate), + Err(_) => { + let data_str = content.to_string(); + let data_str = data_str.unwrap_or_else(|e| format!("")); + (data_str.into(), UpdateType::FullUpdate) + } + }; + Ok(LazyGridDataUpdate { data, update_type }) } } diff --git a/app/gui/view/graph-editor/src/component/node.rs b/app/gui/view/graph-editor/src/component/node.rs index 5c4e52f58ae0..22da8dd792b1 100644 --- a/app/gui/view/graph-editor/src/component/node.rs +++ b/app/gui/view/graph-editor/src/component/node.rs @@ -598,7 +598,7 @@ impl NodeModel { if let Some(error) = error { self.error_visualization.display_kind(*error.kind); if let Some(error_data) = error.visualization_data() { - self.error_visualization.set_data(&error_data); + self.error_visualization.set_data(error_data); } if error.should_display() { self.display_object.add_child(&self.error_visualization); diff --git a/app/gui/view/graph-editor/src/component/visualization/container.rs b/app/gui/view/graph-editor/src/component/visualization/container.rs index d1a5f0ad781c..d7ebb3da0c2d 100644 --- a/app/gui/view/graph-editor/src/component/visualization/container.rs +++ b/app/gui/view/graph-editor/src/component/visualization/container.rs @@ -441,6 +441,7 @@ impl ContainerModel { preprocessor.emit(visualization.on_preprocessor_change.value()); } + #[profile(Debug)] fn set_visualization_data(&self, data: &visualization::Data) { self.visualization.borrow().for_each_ref(|vis| vis.send_data.emit(data)) } @@ -498,6 +499,7 @@ impl ContainerModel { self.view.overlay.is_this_target(target) } + #[profile(Debug)] fn next_visualization( &self, current_vis: &Option, diff --git a/app/gui/view/graph-editor/src/component/visualization/data.rs b/app/gui/view/graph-editor/src/component/visualization/data.rs index 4f71ec143591..2722af335559 100644 --- a/app/gui/view/graph-editor/src/component/visualization/data.rs +++ b/app/gui/view/graph-editor/src/component/visualization/data.rs @@ -10,25 +10,43 @@ use crate::prelude::*; /// Json representation with a fast clone operation. Used for transmitting visualization data via /// FRP networks. -#[derive(Clone, CloneRef, Debug, Default)] +#[derive(Clone, CloneRef, Debug)] pub struct Json { - rc: Rc, + rc: Rc, } -impl Deref for Json { - type Target = serde_json::Value; - fn deref(&self) -> &Self::Target { - &self.rc +impl Default for Json { + fn default() -> Self { + let null = serde_json::Value::default(); + let raw_null = serde_json::value::to_raw_value(&null).unwrap(); + let rc = raw_null.into(); + Self { rc } } } -impl From for Json { - fn from(t: serde_json::Value) -> Self { - let rc = Rc::new(t); - Self { rc } +impl Json { + /// Deserialize the JSON data into an arbitrary type. + #[profile(Debug)] + pub fn deserialize(&self) -> Result { + serde_json::from_str(self.rc.get()).map_err(|_| DataError::InvalidJsonText) + } + + /// Produce a string from the value. If it's a JSON string, return it; otherwise, pretty-print + /// the JSON data. + #[profile(Debug)] + pub fn to_string(&self) -> serde_json::Result { + let data_str: Result = serde_json::from_str(self.rc.get()); + data_str.or_else(|_| serde_json::to_string_pretty(&self.rc)) + } + + /// Return the raw data. + pub fn raw(&self) -> &str { + self.rc.get() } } + + // =================== // === Data Format === // ==================== @@ -87,10 +105,21 @@ impl Default for Data { } } -impl From for Data { - fn from(t: serde_json::Value) -> Self { - let content = t.into(); - Self::Json { content } +impl Data { + /// Deserialize JSON visualization data from the given bytes. + #[profile(Debug)] + pub fn json(data: &[u8]) -> FallibleResult { + let rc: Rc = serde_json::from_slice(data)?; + let content = Json { rc }; + Ok(Self::Json { content }) + } + + /// Get a reference to the contained JSON data. Fails if this is non-JSON (binary) data. + pub fn as_json(&self) -> Result<&Json, DataError> { + match self { + Data::Json { content } => Ok(content), + Data::Binary => Err(DataError::BinaryNotSupported), + } } } diff --git a/app/gui/view/graph-editor/src/component/visualization/foreign/java_script/instance.rs b/app/gui/view/graph-editor/src/component/visualization/foreign/java_script/instance.rs index c43e94455451..5a01d129d28f 100644 --- a/app/gui/view/graph-editor/src/component/visualization/foreign/java_script/instance.rs +++ b/app/gui/view/graph-editor/src/component/visualization/foreign/java_script/instance.rs @@ -201,17 +201,11 @@ impl InstanceModel { #[cfg(not(target_arch = "wasm32"))] fn set_size(&self, _size: Vector2) {} + #[profile(Debug)] #[cfg(target_arch = "wasm32")] fn receive_data(&self, data: &Data) -> result::Result<(), DataError> { - let data_json = match data { - Data::Json { content } => content, - _ => return Err(DataError::BinaryNotSupported), - }; - let data_json: &serde_json::Value = data_json.deref(); - let data_js = match json_to_value(data_json) { - Ok(value) => value, - Err(_) => return Err(DataError::InvalidDataType), - }; + let data_json = data.as_json()?.raw(); + let data_js = js_sys::JSON::parse(data_json).map_err(|_| DataError::InvalidDataType)?; self.try_call1(&self.on_data_received, &data_js) .map_err(|_| DataError::InternalComputationError)?; Ok(()) @@ -227,6 +221,7 @@ impl InstanceModel { self.object.emitPreprocessorChange() } + #[profile(Debug)] #[cfg(target_arch = "wasm32")] /// Helper method to call methods on the wrapped javascript object. fn try_call1( @@ -353,17 +348,3 @@ fn get_method( ) -> Result { Ok(default()) } - -/// Convert the given JSON value to a `JsValue`. -/// -/// Note that we need to use special serializer, as `serde_wasm_bindgen` defaults to outputting -/// some special `Map` type that is not supported by the visualization API (rather than proper -/// objects). -#[cfg(target_arch = "wasm32")] -pub fn json_to_value( - json: &serde_json::Value, -) -> std::result::Result { - use serde::Serialize; - let serializer = serde_wasm_bindgen::Serializer::json_compatible(); - json.serialize(&serializer) -} diff --git a/app/gui/view/graph-editor/src/lib.rs b/app/gui/view/graph-editor/src/lib.rs index 425428b27bdb..4013c41bbe55 100644 --- a/app/gui/view/graph-editor/src/lib.rs +++ b/app/gui/view/graph-editor/src/lib.rs @@ -1824,6 +1824,8 @@ pub struct GraphEditorModel { profiling_statuses: profiling::Statuses, profiling_button: component::profiling::Button, styles_frp: StyleWatchFrp, + #[deprecated = "StyleWatch was designed as an internal tool for shape system (#795)"] + styles: StyleWatch, selection_controller: selection::Controller, execution_environment_selector: ExecutionEnvironmentSelector, sources_of_detached_target_edges: SharedHashSet, @@ -1856,6 +1858,7 @@ impl GraphEditorModel { let drop_manager = ensogl_drop_manager::Manager::new(&scene.dom.root.clone_ref().into(), scene); let styles_frp = StyleWatchFrp::new(&scene.style_sheet); + let styles = StyleWatch::new(&scene.style_sheet); let selection_controller = selection::Controller::new( frp, &app.cursor, @@ -1865,6 +1868,7 @@ impl GraphEditorModel { ); let sources_of_detached_target_edges = default(); + #[allow(deprecated)] // `styles` Self { display_object, app, @@ -1884,6 +1888,7 @@ impl GraphEditorModel { frp: frp.private.clone_ref(), frp_public: frp.public.clone_ref(), styles_frp, + styles, selection_controller, execution_environment_selector, sources_of_detached_target_edges, @@ -2557,17 +2562,17 @@ impl GraphEditorModel { /// This might need to be more sophisticated in the case of polymorphic types. For example, /// consider the edge source type to be `(a,Number)`, and target to be `(Text,a)`. These unify /// to `(Text,Number)`. + #[profile(Debug)] fn edge_color(&self, edge_id: EdgeId, neutral_color: color::Lcha) -> color::Lcha { - // FIXME : StyleWatch is unsuitable here, as it was designed as an internal tool for shape - // system (#795) - let styles = StyleWatch::new(&self.scene().style_sheet); match self.frp_public.output.view_mode.value() { view::Mode::Normal => { let edge_type = self .edge_hover_type() .or_else(|| self.edge_target_type(edge_id)) .or_else(|| self.edge_source_type(edge_id)); - let opt_color = edge_type.map(|t| type_coloring::compute(&t, &styles)); + // FIXME: `type_coloring::compute` should be ported to `StyleWatchFrp`. + #[allow(deprecated)] // `self.styles` + let opt_color = edge_type.map(|t| type_coloring::compute(&t, &self.styles)); opt_color.unwrap_or(neutral_color) } view::Mode::Profiling => neutral_color, diff --git a/lib/rust/ensogl/component/text/src/font.rs b/lib/rust/ensogl/component/text/src/font.rs index b630a9a31057..b4912f316489 100644 --- a/lib/rust/ensogl/component/text/src/font.rs +++ b/lib/rust/ensogl/component/text/src/font.rs @@ -744,6 +744,7 @@ impl FontTemplate { } /// Populate the cache with the given data. + #[profile(Debug)] pub fn load_cache(&self, snapshot: &CacheSnapshot) -> anyhow::Result<()> { self.atlas.set_data(snapshot.atlas.clone()); let cache: HashMap> = diff --git a/lib/rust/frp/src/microtasks.rs b/lib/rust/frp/src/microtasks.rs index 813b293a68d9..038ea70bd470 100644 --- a/lib/rust/frp/src/microtasks.rs +++ b/lib/rust/frp/src/microtasks.rs @@ -186,13 +186,13 @@ impl Scheduler { } fn add(&self, f: impl FnOnce() + 'static) -> callback::Handle { - let handle = self.data.callbacks.add(f); + let handle = self.data.callbacks.add(profile(f)); self.data.schedule_task(); handle } fn add_late(&self, f: impl FnOnce() + 'static) -> callback::Handle { - let handle = self.data.late_callbacks.add(f); + let handle = self.data.late_callbacks.add(profile(f)); self.data.schedule_task(); handle } @@ -202,6 +202,17 @@ impl Scheduler { } } +/// Add profiling to a function. The current profiler (if any) will be the parent, regardless of +/// when the scheduled task is actually executed. +fn profile(f: impl FnOnce() + 'static) -> impl FnOnce() + 'static { + let profiler = profiler::create_debug!(""); + move || { + profiler.resume(); + f(); + profiler.pause(); + } +} + struct SchedulerData { is_scheduled: Cell, callbacks: callback::registry::NoArgsOnce, diff --git a/lib/rust/json-rpc/src/error.rs b/lib/rust/json-rpc/src/error.rs index 47c0f1247ff1..4f11ff69bed9 100644 --- a/lib/rust/json-rpc/src/error.rs +++ b/lib/rust/json-rpc/src/error.rs @@ -79,7 +79,7 @@ pub enum HandlingError { /// Server responded to an identifier that does not match to any known /// ongoing request. #[fail(display = "Server generated a response with no matching request: id={:?}.", _0)] - UnexpectedResponse(Response), + UnexpectedResponse(Response>), /// JSON-RPC client does not expect any binary messages, yet it received one. #[fail(display = "Server sent unexpected binary message: {:?}.", _0)] diff --git a/lib/rust/json-rpc/src/handler.rs b/lib/rust/json-rpc/src/handler.rs index c8a3d8f68ce4..6cdbb773e9d9 100644 --- a/lib/rust/json-rpc/src/handler.rs +++ b/lib/rust/json-rpc/src/handler.rs @@ -1,6 +1,7 @@ //! Module providing `Handler` and related types used by its API. use crate::prelude::*; +use enso_profiler::prelude::*; use crate::api; use crate::api::Result; @@ -13,6 +14,7 @@ use crate::messages::Id; use crate::transport::Transport; use crate::transport::TransportEvent; +use enso_profiler as profiler; use futures::channel::mpsc::unbounded; use futures::channel::mpsc::UnboundedSender; use futures::channel::oneshot; @@ -32,14 +34,15 @@ use std::future::Future; /// Partially decoded reply message. /// /// Known if `Error` or `Success` but returned value remains in JSON form. -pub type ReplyMessage = messages::Result; +pub type ReplyMessage = messages::Result>; /// Converts remote message with JSON-serialized result into `Result`. +#[profile(Debug)] pub fn decode_result( - result: messages::Result, + result: messages::Result>, ) -> Result { match result { - messages::Result::Success(ret) => Ok(serde_json::from_value::(ret.result)?), + messages::Result::Success(ret) => Ok(serde_json::from_str::(ret.result.get())?), messages::Result::Error { error } => Err(RpcError::RemoteError(error)), } } @@ -244,6 +247,7 @@ impl Handler { /// Sends a request to the peer and returns a `Future` that shall yield a /// reply message. It is automatically decoded into the expected type. + #[profile(Debug)] pub fn open_request( &self, input: In, @@ -265,10 +269,11 @@ impl Handler { /// This is suboptimal but still less evil than cloning all input arguments like before. /// /// FIXME: when possible unify with `open_request` + #[profile(Debug)] pub fn open_request_with_json( &self, method_name: &str, - input: &serde_json::Value, + input: &serde_json::value::RawValue, ) -> impl Future> { let id = self.generate_new_id(); let message = crate::messages::Message::new_request(id, method_name, input); @@ -280,7 +285,8 @@ impl Handler { /// /// Helper common \code for `open_request` and `open_request_with_json`. See /// `open_request_with_json` docstring for more information. - pub fn open_request_with_message( + #[profile(Debug)] + fn open_request_with_message( &self, id: Id, message_json: &str, @@ -307,7 +313,8 @@ impl Handler { /// Deal with `Response` message from the peer. /// /// It shall be either matched with an open request or yield an error. - pub fn process_response(&self, message: messages::Response) { + #[profile(Debug)] + pub fn process_response(&self, message: messages::Response>) { if let Some(sender) = self.remove_ongoing_request(message.id) { // Disregard any error. We do not care if RPC caller already // dropped the future. @@ -321,9 +328,14 @@ impl Handler { /// /// If possible, emits a message with notification. In case of failure, /// emits relevant error. - pub fn process_notification(&self, message: messages::Notification) - where Notification: DeserializeOwned { - match serde_json::from_value(message.0) { + #[profile(Debug)] + pub fn process_notification( + &self, + message: messages::Notification>, + ) where + Notification: DeserializeOwned, + { + match serde_json::from_str(message.0.get()) { Ok(notification) => { let event = Event::Notification(notification); self.emit_event(event); @@ -339,6 +351,7 @@ impl Handler { /// /// The message must conform either to the `Response` or to the /// `Notification` JSON-serialized format. Otherwise, an error is raised. + #[profile(Debug)] pub fn process_incoming_message(&self, message: String) where Notification: DeserializeOwned { match messages::decode_incoming_message(&message) { diff --git a/lib/rust/json-rpc/src/lib.rs b/lib/rust/json-rpc/src/lib.rs index 5aaab7e680d0..9fae23490828 100644 --- a/lib/rust/json-rpc/src/lib.rs +++ b/lib/rust/json-rpc/src/lib.rs @@ -33,7 +33,6 @@ pub mod transport; pub use api::RemoteMethodCall; pub use api::Result; -pub use enso_prelude as prelude; pub use enso_profiler; pub use enso_profiler_data; pub use enso_web as ensogl; @@ -45,6 +44,13 @@ pub use transport::TransportEvent; +#[allow(missing_docs)] +pub mod prelude { + pub use enso_prelude::*; + pub use enso_profiler as profiler; + pub use enso_profiler::prelude::*; +} + #[allow(missing_docs)] pub mod constants { use std::time::Duration; diff --git a/lib/rust/json-rpc/src/log.rs b/lib/rust/json-rpc/src/log.rs index d45ac3a6c7bb..a827d19609a3 100644 --- a/lib/rust/json-rpc/src/log.rs +++ b/lib/rust/json-rpc/src/log.rs @@ -1,13 +1,15 @@ //! Interface for logging RPC Requests as [`enso_profiler`] metadata, and interpreting the resultant //! log events. +use crate::prelude::*; + // =========================== // === Rpc Request Logging === // =========================== -enso_profiler::metadata_logger!("RpcRequest", rpc_request(&'static str)); +profiler::metadata_logger!("RpcRequest", rpc_request(&'static str)); @@ -17,10 +19,10 @@ enso_profiler::metadata_logger!("RpcRequest", rpc_request(&'static str)); /// Message sent from the IDE to the Language Server. #[derive(Clone, Debug, serde::Serialize, serde::Deserialize, PartialEq, Eq)] -pub struct RpcRequest(std::borrow::Cow<'static, str>); +pub struct RpcRequest(Cow<'static, str>); -impl std::fmt::Display for RpcRequest { - fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result { +impl Display for RpcRequest { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { f.write_str(&self.0) } } diff --git a/lib/rust/json-rpc/src/macros.rs b/lib/rust/json-rpc/src/macros.rs index 85fd0539c421..dbcb2c3ee71d 100644 --- a/lib/rust/json-rpc/src/macros.rs +++ b/lib/rust/json-rpc/src/macros.rs @@ -100,11 +100,11 @@ macro_rules! make_rpc_methods { json_rpc::log::rpc_request(stringify!($method)); - let phantom = std::marker::PhantomData; - let input = $method_input { phantom, $($param_name:&$param_name),* }; - let input_json = serde_json::to_value(input).unwrap(); - let name = $method_input::NAME; - let result_fut = self.handler.borrow().open_request_with_json(name,&input_json); + let phantom = std::marker::PhantomData; + let input = $method_input { phantom, $($param_name:&$param_name),* }; + let input_json = serde_json::value::to_raw_value(&input).unwrap(); + let name = $method_input::NAME; + let result_fut = self.handler.borrow().open_request_with_json(name, &input_json); profiler.pause(); diff --git a/lib/rust/json-rpc/src/messages.rs b/lib/rust/json-rpc/src/messages.rs index 553c47adbe49..c85a9aa9a133 100644 --- a/lib/rust/json-rpc/src/messages.rs +++ b/lib/rust/json-rpc/src/messages.rs @@ -191,25 +191,55 @@ pub struct Error { /// A message that can come from Server to Client — either a response or /// notification. -#[derive(Serialize, Deserialize, Debug, PartialEq, Eq)] -#[serde(untagged)] +#[derive(Debug)] pub enum IncomingMessage { /// A response to a call made by client. - Response(Response), + Response(Response>), /// A notification call (initiated by the server). - Notification(Notification), + Notification(Notification>), } /// Partially decodes incoming message. /// /// This checks if has `jsonrpc` version string, and whether it is a /// response or a notification. +#[profile(Debug)] pub fn decode_incoming_message(message: &str) -> serde_json::Result { - use serde_json::from_str; - use serde_json::from_value; - use serde_json::Value; - let message = from_str::>(message)?; - from_value::(message.payload) + type Payload = serde_json::value::RawValue; + // We can't use the derived deserialization for `Message` because it is currently incompatible + // with `serde_json::value::RawValue`[1], which is the most performant way to partially-decode + // JSON. + // [1]: (https://github.com/serde-rs/serde/issues/1183) + #[derive(Deserialize, Debug)] + struct RawMessage { + #[allow(dead_code)] // Checked for during deserialization. + jsonrpc: Version, + #[serde(default)] + id: Option, + #[serde(default, deserialize_with = "deserialize_some")] + result: Option>>, + #[serde(default)] + error: Option, + } + fn deserialize_some<'de, T, D>(deserializer: D) -> std::result::Result, D::Error> + where + T: Deserialize<'de>, + D: serde::de::Deserializer<'de>, { + Deserialize::deserialize(deserializer).map(Some) + } + let raw: RawMessage = serde_json::from_str(message)?; + Ok(match (raw.id, raw.result, raw.error) { + (Some(id), Some(result), None) => { + let result = result.unwrap_or_default(); + IncomingMessage::Response(Response { id, result: Result::Success(Success { result }) }) + } + (Some(id), None, Some(error)) => + IncomingMessage::Response(Response { id, result: Result::Error { error } }), + _ => { + let payload: Box = serde_json::from_str(message)?; + IncomingMessage::Notification(Notification(payload)) + } + }) } /// Message from server to client. @@ -317,11 +347,12 @@ mod tests { #[test] fn test_response_deserialization() { let response = r#"{"jsonrpc":"2.0","id":0,"result":{"exists":true}}"#; - let msg = serde_json::from_str(response).unwrap(); + let msg = decode_incoming_message(response).unwrap(); if let IncomingMessage::Response(resp) = msg { assert_eq!(resp.id, Id(0)); if let Result::Success(ret) = resp.result { - let obj = ret.result.as_object().expect("expected object ret"); + let result: Value = serde_json::from_str(ret.result.get()).unwrap(); + let obj = result.as_object().expect("expected object ret"); assert_eq!(obj.len(), 1); let exists = obj.get("exists").unwrap().as_bool().unwrap(); assert!(exists) diff --git a/lib/rust/prelude/src/serde.rs b/lib/rust/prelude/src/serde.rs index 97d22cb7d977..b8912b6bcab7 100644 --- a/lib/rust/prelude/src/serde.rs +++ b/lib/rust/prelude/src/serde.rs @@ -13,8 +13,8 @@ where // We first parse as generic JSON value. This is necessary to consume parser input. // If we just tried parsing the desired type directly and ignored error, we would end up with // `trailing characters` error in non-trivial cases. - let json_value = serde_json::Value::deserialize(d)?; - serde_json::from_value(json_value).or_else(|_error| Ok(Ret::default())) + let raw_json = <&serde_json::value::RawValue>::deserialize(d)?; + serde_json::from_str(raw_json.get()).or_else(|_error| Ok(Ret::default())) } /// Deserialize a JSON value that is either of `Ret` type or equals `null`. A `null` is converted diff --git a/lib/rust/profiler/src/log.rs b/lib/rust/profiler/src/log.rs index de6bff72376d..5ba1b6178f46 100644 --- a/lib/rust/profiler/src/log.rs +++ b/lib/rust/profiler/src/log.rs @@ -14,11 +14,11 @@ //! scope. use std::cell; -use std::mem; /// Allocation unit of events within a [`Log`]. +#[cfg_attr(debug_assertions, allow(dead_code))] const BLOCK: usize = 1024; @@ -27,112 +27,189 @@ const BLOCK: usize = 1024; // === Log === // =========== -/// A shared-mutable data structure supporting append and random-access read. -#[derive(Debug)] -pub struct Log { - current: cell::UnsafeCell; BLOCK]>>, - completed: cell::UnsafeCell>>, - len: cell::Cell, -} +#[cfg(not(debug_assertions))] +pub use fast::Log; -#[allow(unsafe_code)] -impl Log { - /// Create a new, empty [`Log`]. - pub fn new() -> Self { - Self { - current: cell::UnsafeCell::new(Box::new(mem::MaybeUninit::uninit_array())), - completed: cell::UnsafeCell::new(Default::default()), - len: Default::default(), - } +#[cfg(debug_assertions)] +pub use safe::Log; + +/// Fast implementation used when debug assertions are disabled. +#[cfg(not(debug_assertions))] +mod fast { + use super::*; + + use std::mem; + + /// A shared-mutable data structure supporting append and random-access read. + #[derive(Debug)] + pub struct Log { + current: cell::UnsafeCell; BLOCK]>>, + completed: cell::UnsafeCell>>, + len: cell::Cell, } - /// Push an element. - #[inline] - #[allow(unsafe_code)] // Note [Log Safety] - pub fn push(&self, element: T) { - unsafe { - let i = self.len.get(); - (*self.current.get())[i % BLOCK].write(element); - let i1 = i + 1; - if i1 % BLOCK == 0 { - // Current gradually-initialized block is full. Read it, cast it to a - // fully-initialized block, and replace it with a new empty block. - let empty = Box::new(mem::MaybeUninit::uninit_array()); - let block = self.current.get().replace(empty); - let block = - mem::transmute::; BLOCK]>, Box<[T; BLOCK]>>(block); - // Add the old block to our collection of completed blocks. - (*self.completed.get()).push(block); + impl Log { + /// Create a new, empty [`Log`]. + pub fn new() -> Self { + Self { + current: cell::UnsafeCell::new(Box::new(mem::MaybeUninit::uninit_array())), + completed: cell::UnsafeCell::new(Default::default()), + len: Default::default(), } - self.len.set(i1); } - } - /// Returns the number of entries in the log. - #[inline] - pub fn len(&self) -> usize { - self.len.get() - } + /// Push an element. + #[inline] + #[allow(unsafe_code)] // Note [Log Safety] + pub fn push(&self, element: T) { + unsafe { + let i = self.len.get(); + (*self.current.get())[i % BLOCK].write(element); + let i1 = i + 1; + if i1 % BLOCK == 0 { + // Current gradually-initialized block is full. Read it, cast it to a + // fully-initialized block, and replace it with a new empty block. + let empty = Box::new(mem::MaybeUninit::uninit_array()); + let block = self.current.get().replace(empty); + let block = + mem::transmute::; BLOCK]>, Box<[T; BLOCK]>>(block); + // Add the old block to our collection of completed blocks. + (*self.completed.get()).push(block); + } + self.len.set(i1); + } + } - /// Returns true if the log contains no entries. - #[inline] - pub fn is_empty(&self) -> bool { - self.len() == 0 - } + /// Returns the number of entries in the log. + #[inline] + pub fn len(&self) -> usize { + self.len.get() + } - /// Applies a function to each entry in the log. - #[allow(unsafe_code)] // Note [Log Safety] - pub fn for_each(&self, mut f: F) - where F: FnMut(&T) { - unsafe { - let blocks = self.len() / BLOCK; - let n = self.len() % BLOCK; - for i in 0..blocks { - // Safety: The contents of a completed block are never modified, so we can hold a - // borrow while calling the function (which may append to the log). - let block = &(*self.completed.get())[i]; - block.iter().for_each(&mut f); + /// Returns true if the log contains no entries. + #[inline] + pub fn is_empty(&self) -> bool { + self.len() == 0 + } + + /// Applies a function to each entry in the log. + #[allow(unsafe_code)] // Note [Log Safety] + pub fn for_each(&self, mut f: F) + where F: FnMut(&T) { + unsafe { + let blocks = self.len() / BLOCK; + let n = self.len() % BLOCK; + for i in 0..blocks { + // Safety: The contents of a completed block are never modified, so we can hold + // a borrow while calling the function (which may append to + // the log). + let block = &(*self.completed.get())[i]; + block.iter().for_each(&mut f); + } + // Safety: The elements in the completed portion of the block are never modified, so + // we can hold a borrow while calling the function (which may append + // to the log). + let current = &(*self.current.get())[..n]; + current.iter().map(|elem| elem.assume_init_ref()).for_each(f); } - // Safety: The elements in the completed portion of the block are never modified, so we - // can hold a borrow while calling the function (which may append to the log). - let current = &(*self.current.get())[..n]; - current.iter().map(|elem| elem.assume_init_ref()).for_each(f); } - } - #[inline] - #[allow(unsafe_code)] // Note [Log Safety] - fn get(&self, index: usize) -> Option<&T> { - unsafe { - let block_i = index / BLOCK; - let i = index % BLOCK; - let blocks = &*self.completed.get(); - if let Some(block) = blocks.get(block_i) { - Some(&block[i]) - } else if block_i == blocks.len() && i < self.len.get() % BLOCK { - Some((*self.current.get())[i].assume_init_ref()) - } else { - None + /// Pass the element at the specified index to the given function. Returns `None` if the + /// index is out of bounds. + #[inline] + #[allow(unsafe_code)] // Note [Log Safety] + pub fn get(&self, index: usize, f: impl FnOnce(&T) -> U) -> Option { + unsafe { + let block_i = index / BLOCK; + let i = index % BLOCK; + let blocks = &*self.completed.get(); + let value = if let Some(block) = blocks.get(block_i) { + Some(&block[i]) + } else if block_i == blocks.len() && i < self.len.get() % BLOCK { + Some((*self.current.get())[i].assume_init_ref()) + } else { + None + }; + // Safety: Whether the element is in a completed block, or in the completed portion + // of the current block, it will never be moved or modified; so we + // can hold a borrow while calling the function (which may append to + // the log). + value.map(f) } } } -} -impl Log { - /// Return a collection of all entries currently in the log. - pub fn clone_all(&self) -> C - where C: Default + Extend { - let mut result = C::default(); - self.for_each(|elem| result.extend_one(elem.clone())); - result + impl Log { + /// Return a collection of all entries currently in the log. + pub fn clone_all(&self) -> C + where C: Default + Extend { + let mut result = C::default(); + self.for_each(|elem| result.extend_one(elem.clone())); + result + } } } -impl core::ops::Index for Log { - type Output = T; - #[inline] - fn index(&self, index: usize) -> &Self::Output { - self.get(index).unwrap() +/// Checked implementation used when debug assertions are enabled. +#[cfg(debug_assertions)] +mod safe { + use super::*; + + /// A shared-mutable data structure supporting append and random-access read. + #[derive(Debug)] + pub struct Log { + data: cell::RefCell>, + } + + impl Log { + /// Create a new, empty [`Log`]. + #[inline] + pub fn new() -> Self { + let data = cell::RefCell::new(Vec::new()); + Self { data } + } + + /// Push an element. + #[inline] + pub fn push(&self, element: T) { + self.data.borrow_mut().push(element) + } + + /// Returns the number of entries in the log. + #[inline] + pub fn len(&self) -> usize { + self.data.borrow().len() + } + + /// Returns true if the log contains no entries. + #[inline] + pub fn is_empty(&self) -> bool { + self.len() == 0 + } + + /// Applies a function to each entry in the log. + #[inline] + pub fn for_each(&self, f: F) + where F: FnMut(&T) { + self.data.borrow().iter().for_each(f) + } + + /// Pass the element at the specified index to the given function. Returns `None` if the + /// index is out of bounds. + #[inline] + pub fn get(&self, index: usize, f: impl FnOnce(&T) -> U) -> Option { + self.data.borrow().get(index).map(f) + } + } + + impl Log { + /// Return a collection of all entries currently in the log. + pub fn clone_all(&self) -> C + where C: Default + Extend { + let mut result = C::default(); + result.extend(self.data.borrow().iter().cloned()); + result + } } } @@ -182,7 +259,7 @@ impl ThreadLocalLog { /// /// Panics if the index is not less than [`len`]. pub fn get(&'static self, i: usize, f: impl FnOnce(&T) -> U) -> U { - self.0.with(|this| f(&this[i])) + self.try_get(i, f).unwrap() } /// Get the entry at the given index, and pass it to a function; return the result of the @@ -190,7 +267,7 @@ impl ThreadLocalLog { /// /// Returns [`None`] if the index is not less than [`len`]. pub fn try_get(&'static self, i: usize, f: impl FnOnce(&T) -> U) -> Option { - self.0.with(|this| this.get(i).map(f)) + self.0.with(|this| this.get(i, f)) } }