Merge pull request #836 from savetheclocktower/fix-scope-resolver-config-cache

[tree-sitter] Share config caches between `ScopeResolver`s

Author: savetheclocktower

packages/language-clojure/grammars/ts/highlights.scm

@@ -34,9 +34,12 @@
 ((list_lit
   "(" @punctuation.section.expression.begin (#is-not? test.descendantOfNodeWithData "clojure.dismissTag")
   .
-  (sym_lit) @meta.definition.global @keyword.control (#eq? @meta.definition.global "ns")
+  (sym_lit) @meta.definition.global @keyword.control
+    (#eq? @meta.definition.global "ns")
   .
-  (sym_lit) @meta.definition.global @entity.global
+  ; We need to distinguish this `@meta.definition.global` from the one above or
+  ; else this query will fail.
+  (sym_lit) @meta.definition.global.__TEXT__ @entity.global
   ")" @punctuation.section.expression.end)
  @meta.namespace.clojure
  (#set! isNamespace true))

packages/language-clojure/package.json

@@ -5,7 +5,7 @@
   "main": "lib/main",
   "engines": {
     "atom": "*",
-    "node": "*"
+    "node": ">=12"
   },
   "repository": "https://github.com/pulsar-edit/pulsar",
   "license": "MIT",

spec/scope-resolver-spec.js

@@ -32,13 +32,9 @@ const jsRegexGrammarPath = resolve(
 );
 let jsRegexConfig = CSON.readFileSync(jsRegexGrammarPath);
 
-async function getAllCaptures(grammar, languageMode, layer = null) {
+async function getAllCapturesWithScopeResolver(grammar, languageMode, scopeResolver, layer = null) {
   let query = await grammar.getQuery('highlightsQuery');
   layer = layer ?? languageMode.rootLanguageLayer;
-  let scopeResolver = new ScopeResolver(
-    layer,
-    (name) => languageMode.idForScope(name),
-  );
   let { start, end } = languageMode.buffer.getRange();
   let { tree } = layer;
   return {
@@ -47,6 +43,32 @@ async function getAllCaptures(grammar, languageMode, layer = null) {
   };
 }
 
+function makeScopeResolver(languageMode, layer) {
+  layer = layer ?? languageMode.rootLanguageLayer;
+  return new ScopeResolver(
+    layer,
+    (name) => languageMode.idForScope(name),
+  );
+}
+
+async function getAllCaptures(grammar, languageMode, layer = null) {
+  layer = layer ?? languageMode.rootLanguageLayer;
+  let scopeResolver = makeScopeResolver(languageMode, layer);
+  return getAllCapturesWithScopeResolver(grammar, languageMode, scopeResolver, layer);
+}
+
+async function getAllMatchesWithScopeResolver(...args) {
+  let { captures, scopeResolver } = await getAllCapturesWithScopeResolver(...args);
+  let matches = [];
+  for (let capture of captures) {
+    let range = scopeResolver.store(capture);
+    if (range) {
+      matches.push(capture);
+    }
+  }
+  return matches;
+}
+
 async function getAllMatches(...args) {
   let { captures, scopeResolver } = await getAllCaptures(...args);
   let matches = [];
@@ -1136,12 +1158,33 @@ describe('ScopeResolver', () => {
       `);
       await languageMode.ready;
 
-      let matched = await getAllMatches(grammar, languageMode);
+      let scopeResolver = makeScopeResolver(languageMode);
+
+      let matched = await getAllMatchesWithScopeResolver(grammar, languageMode, scopeResolver);
       expect(matched.length).toBe(4);
 
       atom.config.set('core.careAboutBooleans', "something-else");
 
-      matched = await getAllMatches(grammar, languageMode);
+      matched = await getAllMatchesWithScopeResolver(grammar, languageMode, scopeResolver);
+      expect(matched.length).toBe(0);
+
+      atom.config.set(
+        'core.careAboutBooleans',
+        'something',
+        { scope: [grammar.scopeName] }
+      );
+
+      matched = await getAllMatchesWithScopeResolver(grammar, languageMode, scopeResolver);
+      expect(matched.length).toBe(4);
+
+      atom.config.set('core.careAboutBooleans', "something");
+
+      atom.config.set(
+        'core.careAboutBooleans',
+        'something-else',
+        { scope: [grammar.scopeName] }
+      );
+      matched = await getAllMatchesWithScopeResolver(grammar, languageMode, scopeResolver);
       expect(matched.length).toBe(0);
     });
 

src/scope-resolver.js

@@ -1,3 +1,4 @@
+const { CompositeDisposable } = require('event-kit');
 const { Point } = require('text-buffer');
 const ScopeDescriptor = require('./scope-descriptor');
 
@@ -16,7 +17,7 @@ function comparePoints(a, b) {
   }
 }
 
-function rangeSpecToString (range) {
+function rangeSpecToString(range) {
   let [sp, ep] = [range.startPosition, range.endPosition];
   return `(${sp.row}, ${sp.column}) - (${ep.row}, ${ep.column})`;
 }
@@ -63,6 +64,67 @@ function interpretPossibleKeyValuePair(rawValue, coerceValue = false) {
   return [key, value];
 }
 
+// `ScopeResolver`s can share a config cache if they have the same grammar.
+// There are many such `ScopeResolver`s (because there are many such
+// `LanguageLayer`s), and this consolidation cuts down on the number of
+// `onDidChange` handlers (which are costly when observing all config values).
+class ConfigCache {
+  constructor(config) {
+    this.subscriptions = new CompositeDisposable();
+    this.cachesByGrammar = new Map();
+    this.config = config;
+
+    this.subscriptions.add(
+      this.config.onDidChange(() => this.clearAll()),
+      atom.grammars.onDidAddGrammar(() => this.clearAll()),
+      atom.grammars.onDidUpdateGrammar(() => this.clearAll())
+    );
+  }
+
+  dispose() {
+    this.subscriptions.dispose();
+  }
+
+  clearAll() {
+    for (let cache of this.cachesByGrammar.values()) {
+      cache.clear();
+    }
+  }
+
+  getCacheForGrammar(grammar) {
+    let { scopeName } = grammar;
+    // We key on the scope name rather than the grammar instance. We need to be
+    // able to iterate over grammars, so we can't use a `WeakSet` here, and we
+    // have no lifecycle event to keep the map from getting stale.
+    //
+    // To prevent two different incarnations of the same grammar (after
+    // disabling and reenabling) from incorrectly sharing a cache, we clear all
+    // caches whenever grammars are added or updated.
+    let cache = this.cachesByGrammar.get(scopeName);
+    if (!cache) {
+      cache = new Map();
+      this.cachesByGrammar.set(scopeName, cache);
+    }
+    return cache;
+  }
+}
+
+// We can technically have more than one configuration object, though in
+// practice this will only point to `atom.config`. We do it this way for ease
+// of testing (e.g., if a test mocks a config object) and to avoid silly hacks.
+ConfigCache.CACHES_FOR_CONFIG_OBJECTS = new Map();
+
+ConfigCache.forConfig = (config) => {
+  let { CACHES_FOR_CONFIG_OBJECTS } = ConfigCache;
+  let configCache = CACHES_FOR_CONFIG_OBJECTS.get(config);
+  if (!configCache) {
+    configCache = new ConfigCache(config);
+    CACHES_FOR_CONFIG_OBJECTS.set(config, configCache);
+  }
+  return configCache;
+};
+
+
 // A data structure for storing scope information while processing capture
 // data. The data is reset in between each task.
 //
@@ -86,10 +148,8 @@ class ScopeResolver {
     this.rangeData = new Map;
     this.pointKeyCache = new Map;
     this.patternCache = new Map;
-    this.configCache = new Map;
-    this.configSubscription = this.config.onDidChange(() => {
-      this.configCache.clear();
-    });
+    this.configCache = ConfigCache.forConfig(this.config)
+      .getCacheForGrammar(this.grammar);
   }
 
   getOrCompilePattern(pattern) {
@@ -477,8 +537,6 @@ class ScopeResolver {
     this.reset();
     this.patternCache.clear();
     this.pointKeyCache.clear();
-    this.configCache.clear();
-    this.configSubscription.dispose();
   }
 
   *[Symbol.iterator]() {