From fc03b3faa1e9c383035dc9b95fac58d114840401 Mon Sep 17 00:00:00 2001 From: Tor Hovland Date: Thu, 13 Jul 2023 14:05:05 +0200 Subject: [PATCH] Keep query between runs when we can in the playground, for performance. (#560) * Static query state. * Logic rework in the React app. * Comments and cleanup. * Update test. --- Cargo.lock | 14 +-- topiary-playground/src/lib.rs | 92 ++++++++++++------- web-playground/e2e/sample-tester.test.ts | 2 +- web-playground/src/App.tsx | 112 +++++++++++++++++------ 4 files changed, 153 insertions(+), 67 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index ff28ca1d..52408e58 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -622,7 +622,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "cb0889898416213fab133e1d33a0e5858a48177452750691bde3666d0fdbaf8b" dependencies = [ "hermit-abi", - "rustix 0.38.3", + "rustix 0.38.4", "windows-sys", ] @@ -933,9 +933,9 @@ dependencies = [ [[package]] name = "regex-automata" -version = "0.3.2" +version = "0.3.3" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "83d3daa6976cffb758ec878f108ba0e062a45b2d6ca3a2cca965338855476caf" +checksum = "39354c10dd07468c2e73926b23bb9c2caca74c5501e38a35da70406f1d923310" dependencies = [ "aho-corasick", "memchr", @@ -970,9 +970,9 @@ dependencies = [ [[package]] name = "rustix" -version = "0.38.3" +version = "0.38.4" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "ac5ffa1efe7548069688cd7028f32591853cd7b5b756d41bcffd2353e4fc75b4" +checksum = "0a962918ea88d644592894bc6dc55acc6c0956488adcebbfb6e273506b7fd6e5" dependencies = [ "bitflags 2.3.3", "errno", @@ -1033,9 +1033,9 @@ dependencies = [ [[package]] name = "serde_json" -version = "1.0.100" +version = "1.0.102" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "0f1e14e89be7aa4c4b78bdbdc9eb5bf8517829a600ae8eaa39a6e1d960b5185c" +checksum = "b5062a995d481b2308b6064e9af76011f2921c35f97b0468811ed9f6cd91dfed" dependencies = [ "itoa", "ryu", diff --git a/topiary-playground/src/lib.rs b/topiary-playground/src/lib.rs index 1d7e6a06..c5b5228e 100644 --- a/topiary-playground/src/lib.rs +++ b/topiary-playground/src/lib.rs @@ -1,10 +1,24 @@ #[cfg(target_arch = "wasm32")] -use topiary::{formatter, Configuration, FormatterResult, Operation, TopiaryQuery}; +use std::sync::Mutex; +#[cfg(target_arch = "wasm32")] +use topiary::{formatter, Configuration, FormatterResult, Language, Operation, TopiaryQuery}; #[cfg(target_arch = "wasm32")] use tree_sitter_facade::TreeSitter; #[cfg(target_arch = "wasm32")] use wasm_bindgen::prelude::*; +#[cfg(target_arch = "wasm32")] +struct QueryState { + language: Language, + grammar: tree_sitter_facade::Language, + query: TopiaryQuery, +} + +#[cfg(target_arch = "wasm32")] +/// The query state is stored in a static variable, so the playground can reuse +/// it across multiple runs as long as it doesn't change. +static QUERY_STATE: Mutex> = Mutex::new(None); + #[cfg(target_arch = "wasm32")] #[wasm_bindgen(js_name = topiaryInit)] pub async fn topiary_init() -> Result<(), JsError> { @@ -17,55 +31,69 @@ pub async fn topiary_init() -> Result<(), JsError> { TreeSitter::init().await } +#[cfg(target_arch = "wasm32")] +#[wasm_bindgen(js_name = queryInit)] +pub async fn query_init(query_content: String, language_name: String) -> Result<(), JsError> { + let language_normalized = language_name.replace('-', "_"); + let configuration = Configuration::parse_default_configuration()?; + let language = configuration.get_language(language_normalized)?.clone(); + let grammar = language.grammar_wasm().await?; + let query = TopiaryQuery::new(&grammar, &query_content)?; + + let mut guard = QUERY_STATE.lock().unwrap(); + + *guard = Some(QueryState { + language, + grammar, + query, + }); + + Ok(()) +} + #[cfg(target_arch = "wasm32")] #[wasm_bindgen] pub async fn format( input: &str, - query: &str, - language: &str, check_idempotence: bool, tolerate_parsing_errors: bool, ) -> Result { - let language_normalized = language.replace('-', "_"); - format_inner( - input, - query, - language_normalized.as_str(), - check_idempotence, - tolerate_parsing_errors, - ) - .await - .map_err(|e| format_error(&e)) + format_inner(input, check_idempotence, tolerate_parsing_errors) + .await + .map_err(|e| format_error(&e)) } #[cfg(target_arch = "wasm32")] async fn format_inner( input: &str, - query_content: &str, - language_name: &str, check_idempotence: bool, tolerate_parsing_errors: bool, ) -> FormatterResult { let mut output = Vec::new(); - let configuration = Configuration::parse_default_configuration()?; - let language = configuration.get_language(language_name)?; - let grammar = language.grammar_wasm().await?; - let query = TopiaryQuery::new(&grammar, query_content)?; + let mut guard = QUERY_STATE.lock().unwrap(); - formatter( - &mut input.as_bytes(), - &mut output, - &query, - language, - &grammar, - Operation::Format { - skip_idempotence: !check_idempotence, - tolerate_parsing_errors, - }, - )?; - - Ok(String::from_utf8(output)?) + match &mut *guard { + Some(query_state) => { + formatter( + &mut input.as_bytes(), + &mut output, + &query_state.query, + &query_state.language, + &query_state.grammar, + Operation::Format { + skip_idempotence: !check_idempotence, + tolerate_parsing_errors, + }, + )?; + + Ok(String::from_utf8(output)?) + } + None => Err(topiary::FormatterError::Internal( + "The query has not been initialized.".into(), + None, + )), + } } #[cfg(target_arch = "wasm32")] diff --git a/web-playground/e2e/sample-tester.test.ts b/web-playground/e2e/sample-tester.test.ts index 638c08a7..514c83b4 100644 --- a/web-playground/e2e/sample-tester.test.ts +++ b/web-playground/e2e/sample-tester.test.ts @@ -144,7 +144,7 @@ async function readOutput() { // Wait for useful output. await page.waitForFunction( - el => el?.textContent !== "" && el?.textContent !== "Formatting ...", + el => el?.textContent !== "" && el?.textContent !== "Formatting ..." && el?.textContent !== "Compiling query ...", { polling: "mutation", timeout: 30000 }, el ); diff --git a/web-playground/src/App.tsx b/web-playground/src/App.tsx index 459ce739..338d44a7 100644 --- a/web-playground/src/App.tsx +++ b/web-playground/src/App.tsx @@ -4,6 +4,7 @@ import useDebounce from "./hooks/useDebounce"; import languages from './samples/languages_export'; import init, { topiaryInit, + queryInit, format, } from "./wasm-app/topiary_playground.js"; import "./App.css"; @@ -11,11 +12,19 @@ import "./App.css"; const debounceDelay = 500; function App() { - const [isInitialised, setIsInitialised] = useState(false); - const initCalled = useRef(false); const defaultLanguage = "json"; const defaultQuery = languages[defaultLanguage].query; const defaultInput = languages[defaultLanguage].input; + + // These don't have to be useState, as they don't need to trigger UI changes. + const initCalled = useRef(false); + const isQueryCompiling = useRef(false); + const queryChanged = useRef(true); + const previousDebouncedInput = useRef(""); + const previousDebouncedQuery = useRef(""); + const previousIsInitialised = useRef(false); + + const [isInitialised, setIsInitialised] = useState(false); const [languageOptions, setLanguageOptions] = useState([] as ReactElement[]); const [currentLanguage, setCurrentLanguage] = useState(defaultLanguage); const [onTheFlyFormatting, setOnTheFlyFormatting] = useState(true); @@ -31,26 +40,6 @@ function App() { const debouncedInput = useDebounce(input, debounceDelay); const debouncedQuery = useDebounce(query, debounceDelay); - const runFormat = useCallback((i: string, q: string) => { - const outputFormat = async () => { - try { - const start = performance.now(); - setOutput(await format(i, q, currentLanguage, idempotence, tolerateParsingErrors)); - setProcessingTime(performance.now() - start); - } catch (e) { - setOutput(String(e)); - } - } - - if (!isInitialised) { - setOutput("Cannot format yet, as the formatter engine is being initialised. Try again soon."); - return; - } - - setOutput("Formatting ..."); - outputFormat(); - }, [currentLanguage, idempotence, tolerateParsingErrors, isInitialised]); - // Init page (runs only once, but twice in strict mode in dev) useEffect(() => { const initWasm = async () => { @@ -58,8 +47,8 @@ function App() { if (initCalled.current) return; initCalled.current = true; - await init(); - await topiaryInit(); + await init(); // Does the WebAssembly.instantiate() + await topiaryInit(); // Does the TreeSitter::init() setIsInitialised(true); } @@ -78,10 +67,78 @@ function App() { .catch(console.error); }, []); + // EsLint react-hooks/exhaustive-deps: + // A 'runFormat' function would make the dependencies of the useEffect Hook + // below change on every render. To fix this, we wrap the definition of + // 'runFormat' in its own useCallback() Hook. + const runFormat = useCallback(() => { + if (!isInitialised) { + setOutput("Cannot format yet, as the formatter engine is being initialised. Try again soon."); + return; + } + + if (isQueryCompiling.current) { + setOutput("Query is being compiled. Try again soon."); + return; + } + + // This is how to run async within useEffect and useCallback. + // https://devtrium.com/posts/async-functions-useeffect + const outputFormat = async () => { + try { + if (queryChanged.current) { + isQueryCompiling.current = true; + setOutput("Compiling query ..."); + await queryInit(query, currentLanguage); + queryChanged.current = false; + isQueryCompiling.current = false; + } + + setOutput("Formatting ..."); + setOutput(await format(input, idempotence, tolerateParsingErrors)); + setProcessingTime(performance.now() - start); + } catch (e) { + queryChanged.current = false; + isQueryCompiling.current = false; + setOutput(String(e)); + } + } + + const start = performance.now(); + outputFormat(); + }, [currentLanguage, idempotence, isInitialised, tolerateParsingErrors, input, query]); + // Run on every (debounced) input change, as well as when isInitialised is set. useEffect(() => { if (!onTheFlyFormatting) return; - runFormat(debouncedInput, debouncedQuery); + + // This is how to run async within useEffect and useCallback. + // https://devtrium.com/posts/async-functions-useeffect + const run = async () => { + await runFormat(); + } + + // We don't want to run whenever a dependency changes, but only when either of these do: + if (previousDebouncedInput.current !== debouncedInput || + previousDebouncedQuery.current !== debouncedQuery || + previousIsInitialised.current !== isInitialised) { + if (!isInitialised) { + setOutput("Cannot format yet, as the formatter engine is being initialised. Try again soon."); + return; + } + + if (isQueryCompiling.current) { + setOutput("Query is being compiled. Try again soon."); + return; + } + + run() + .catch(console.error); + } + + previousDebouncedInput.current = debouncedInput; + previousDebouncedQuery.current = debouncedQuery; + previousIsInitialised.current = isInitialised; }, [isInitialised, debouncedInput, debouncedQuery, onTheFlyFormatting, runFormat]) function changeLanguage(l: string) { @@ -95,6 +152,7 @@ function App() { if (!hasModification || window.confirm(confirmationMessage)) { setInput(languages[l].input); setQuery(languages[l].query); + queryChanged.current = true; setOutput(""); setCurrentLanguage(l); } @@ -102,7 +160,7 @@ function App() { } function handleFormat() { - runFormat(input, query); + runFormat(); }; function handleOnTheFlyFormatting() { @@ -152,7 +210,7 @@ function App() {

Query

- setQuery(s)} placeholder="Enter your query here ..." /> + { setQuery(s); queryChanged.current = true; }} placeholder="Enter your query here ..." />

Input