Skip to content

perf(types): точечная инвалидация memo членов вместо глобальной эпохи#4280

Open
nixel2007 wants to merge 7 commits into
developfrom
perf/type-members-per-document-invalidation
Open

perf(types): точечная инвалидация memo членов вместо глобальной эпохи#4280
nixel2007 wants to merge 7 commits into
developfrom
perf/type-members-per-document-invalidation

Conversation

@nixel2007

@nixel2007 nixel2007 commented Jul 16, 2026

Copy link
Copy Markdown
Member

На пакетном анализе AnalyzeCommand.getFileInfoFromFile перестраивает каждый документ (rebuildDocument), поэтому на каждый файл летел DocumentContextContentChangedEvent, а TypeRegistry.invalidateMembersCache глобально сдвигал membersEpoch — обесценивая memo членов всех типов и name-индекс глобальной области. При параллельном анализе один поток сносил кэш посреди резолва другого, из-за чего getMembers и globalNameIndex пересобирались тысячи раз вместо одного.

Что сделано

Инвалидация memo членов на правку документа теперь точечная — и для BSL, и для OScript, согласованно с per-document моделью прочих индексов системы типов (InferredExpressionTypeIndex и др.), где правка документа сбрасывает кэш только этого документа.

  • BSL (ConfigurationModuleMembersProvider): сбрасывается memo только затронутого типа (TypeRegistry.invalidateMembers); общий модуль дополнительно освежает член GLOBAL_CONTEXT.
  • OScript (OScriptModuleMembersProvider): сбрасывается memo правленого документа и его наследников транзитивно — обход TypeRelations.subtypes вширь. Обход обязан быть транзитивным: inheritedMembers копирует к себе результат getMembers родителя, поэтому в memo наследника лежит снимок членов родителя, и сброса одного родителя мало — снимок протухает на всю глубину иерархии. Реализаторов интерфейсов обходить не нужно: &Реализует членов не приносит. Отсечка по URI защищает и от циклов в объявлениях наследования.
  • Глобальный листенер TypeRegistry.invalidateMembersCache удалён целиком — после перевода обоих языков на точечную инвалидацию ему нечего делать. membersEpoch теперь версионирует ровно структурные мутации реестра.
  • Name-индекс глобальной области больше не держит отдельный счётчик поколения: он кэшируется по идентичности наборов-источников (getMembers отдаёт тот же экземпляр списка, пока memo живо, и новый — после любой инвалидации).
  • Гонка «публикация устаревшего результата» (параллельный незавершённый computeMembers дописывает в кэш уже после инвалидации) закрыта пер-типовым поколением: getMembers штампует запись поколением, снятым до вычисления, и отвергает устаревшую публикацию.

Замеры

Надёжная метрика — детерминированный счётчик вызовов computeMembers (пересборок memo), не CPU-сэмплирование:

  • BSL, cpm (подмножество Catalogs, 2071 файл, 440 различных типов): computeMembers 25981 → 496, globalNameIndex 10440 → 14.
  • OScript, autumn-library/winow (2180 файлов, одинаковый объём работы в обоих прогонах): computeMembers 16044 → 769 (~21×).
  • JMH (MembersCacheLookup): пер-типовое поколение добавляет к попаданию в memo ~1.9 нс на пустой карте поколений и ~3.4 нс на заполненной; реальный getMembers на попадании — 5.7/8.4 нс. На масштабе прогона это десятки миллисекунд.

Wall-time на профилировочной машине слишком шумит, чтобы заявлять проценты (тот же jar давал разброс до 1.8×), поэтому эффект показан по числу пересборок.

Claude-Session: https://claude.ai/code/session_01HLvkKQCwBjTHwuZ4EGBQK6

Описание

Связанные задачи

Closes

Чеклист

Общие

  • Ветка PR обновлена из develop
  • Отладочные, закомментированные и прочие, не имеющие смысла участки кода удалены
  • Изменения покрыты тестами
  • Обязательные действия перед коммитом выполнены (запускал команду gradlew precommit)

Для диагностик

  • Описание диагностики заполнено для обоих языков (присутствуют файлы для обоих языков, для русского заполнено все подробно, перевод на английский можно опустить)

Дополнительно

Summary by CodeRabbit

  • Bug Fixes
    • Improved member-cache invalidation to refresh only the affected module/type after rebuilds or document edits, keeping unrelated types unchanged.
    • Strengthened global scope indexing so name lookups rebuild correctly when underlying sources change.
    • Ensured safe cache behavior when invalidation occurs during in-progress recomputation, avoiding stale results.
  • Performance
    • Added a benchmark to measure member-cache lookup and cache-hit overhead.
  • Tests
    • Added/updated tests for scoped invalidation behavior and transitive OS inheritance member rebuilds.

На пакетном анализе AnalyzeCommand.getFileInfoFromFile перестраивает каждый
документ (rebuildDocument), поэтому на каждый из 13090 файлов летел
DocumentContextContentChangedEvent, а TypeRegistry.invalidateMembersCache
глобально сдвигал membersEpoch — обесценивая memo членов ВСЕХ типов и
эпоха-кэшированный name-индекс глобальной области. При параллельном анализе
один поток сносил кэш посреди резолва другого, из-за чего getMembers и
globalNameIndex пересобирались тысячи раз вместо одного.

Замер на cpm (подмножество Catalogs, 2071 файл): computeMembers 25981→496
вызовов на 440 различных типов, globalNameIndex 10440→14. Фаза анализа полной
конфигурации: computeMembers 7.5%→0%, globalNameIndex 7.8%→0% CPU,
wall 7:26→6:27 (−13%).

Инвалидация на изменение содержимого BSL-документа теперь точечная — только
по затронутому типу (TypeRegistry.invalidateMembers), без сдвига глобальной
эпохи; общий модуль дополнительно освежает член GLOBAL_CONTEXT и name-индекс
(GlobalScopeProvider.invalidateNameIndex). Это согласовано с per-document
моделью инвалидации прочих индексов системы типов (InferredExpressionTypeIndex
и др.), где правка документа сбрасывает кэш только этого документа.

OScript оставлен на широкой инвалидации через эпоху: типы .os связаны
межфайловым наследованием (&Расширяет), правка родителя меняет унаследованные
члены наследника в другом документе. registerGlobalPropertyType сдвигает эпоху
только при реальном росте набора типов, а не на каждую повторную пометку.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HLvkKQCwBjTHwuZ4EGBQK6
@coderabbitai

coderabbitai Bot commented Jul 16, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

Module and global-scope member caches now use targeted generation-based invalidation. Registry epoch updates are narrowed, module rebuilds refresh affected caches, OScript inheritance changes propagate invalidation to subtypes, and benchmarks and tests cover the updated behavior.

Changes

Member cache invalidation

Layer / File(s) Summary
Selective registry cache invalidation
src/main/java/.../TypeRegistry.java, src/test/java/.../TypeRegistryScopedSourcesTest.java
Per-type generations support targeted eviction and stale-computation rejection; global-property updates and document-event invalidation behavior are narrowed and tested.
Module rebuild and global index refresh
src/main/java/.../ConfigurationModuleMembersProvider.java, src/main/java/.../GlobalScopeProvider.java, src/test/java/.../ConfigurationModuleMembersProviderTest.java
Module rebuilds invalidate module and global-context members, while the global name index tracks current member sources; tests verify scoped refreshes.
OScript inheritance invalidation
src/main/java/.../OScriptModuleMembersProvider.java, src/test/java/.../OScriptInheritanceMembersTest.java
Document changes traverse transitive subtypes and invalidate their member caches, with coverage for grandchild refreshes.
Cache lookup benchmark support
src/jmh/.../MembersCacheLookup.java, build.gradle.kts
Adds benchmarks for epoch-only and generation-aware cache checks and enables ZIP64 for the JMH archive.

Estimated code review effort: 3 (Moderate) | ~30 minutes

Sequence Diagram(s)

sequenceDiagram
  participant DocumentContextContentChangedEvent
  participant OScriptModuleMembersProvider
  participant TypeRelations
  participant TypeRegistry
  participant GlobalScopeProvider
  DocumentContextContentChangedEvent->>OScriptModuleMembersProvider: notify changed document
  OScriptModuleMembersProvider->>TypeRelations: resolve transitive subtypes
  OScriptModuleMembersProvider->>TypeRegistry: invalidate affected TypeRefs
  GlobalScopeProvider->>TypeRegistry: read current member sources
  GlobalScopeProvider->>GlobalScopeProvider: rebuild global name index when sources change
Loading

Possibly related PRs

Suggested reviewers: claude

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed Docstring coverage is 81.82% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed Титул точно отражает основное изменение: переход от глобальной эпохи к точечной инвалидации memo членов.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch perf/type-members-per-document-invalidation

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (1)
src/test/java/com/github/_1c_syntax/bsl/languageserver/types/registry/ConfigurationModuleMembersProviderTest.java (1)

63-76: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Assert that the edited module’s cached list was actually replaced.

The final content assertion also passes if module invalidation never occurs. Capture the module members before publishing the event and assert the subsequent collection is not the same instance.

Proposed test strengthening
-    assertThat(typeRegistry.getMembers(moduleType, FileType.BSL))
+    var moduleMembersBefore = typeRegistry.getMembers(moduleType, FileType.BSL);
+    assertThat(moduleMembersBefore)
       .extracting(member -> member.name()).contains("НеУстаревшаяПроцедура");
...
-    assertThat(typeRegistry.getMembers(moduleType, FileType.BSL))
+    var moduleMembersAfter = typeRegistry.getMembers(moduleType, FileType.BSL);
+    assertThat(moduleMembersAfter).isNotSameAs(moduleMembersBefore);
+    assertThat(moduleMembersAfter)
       .extracting(member -> member.name()).contains("НеУстаревшаяПроцедура");

As per coding guidelines, maintain or improve test coverage using appropriate test frameworks.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@src/test/java/com/github/_1c_syntax/bsl/languageserver/types/registry/ConfigurationModuleMembersProviderTest.java`
around lines 63 - 76, Strengthen the test around
ConfigurationModuleMembersProviderTest by capturing the edited module’s member
collection before publishing DocumentContextContentChangedEvent, then assert the
collection returned afterward is not the same instance while preserving the
existing content assertion. Keep the arrayType same-instance assertion
unchanged.

Source: Coding guidelines

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In
`@src/main/java/com/github/_1c_syntax/bsl/languageserver/types/registry/TypeRegistry.java`:
- Around line 608-611: Make targeted invalidation generation-based: in
TypeRegistry.invalidateMembers, increment a canonical per-type generation keyed
by the resolved source so alternate TypeRefs share it, and have getMembers
capture and validate that generation before publishing cached results. In
GlobalScopeProvider at the specified site, include the GLOBAL_CONTEXT generation
in GlobalIndex and reject publication from computations created under an older
generation. Apply the TypeRegistry change in TypeRegistry.java:608-611 and the
GlobalIndex change in GlobalScopeProvider.java:175-176.

---

Nitpick comments:
In
`@src/test/java/com/github/_1c_syntax/bsl/languageserver/types/registry/ConfigurationModuleMembersProviderTest.java`:
- Around line 63-76: Strengthen the test around
ConfigurationModuleMembersProviderTest by capturing the edited module’s member
collection before publishing DocumentContextContentChangedEvent, then assert the
collection returned afterward is not the same instance while preserving the
existing content assertion. Keep the arrayType same-instance assertion
unchanged.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 4218a74b-ed83-417f-a98c-9785edf3bfd5

📥 Commits

Reviewing files that changed from the base of the PR and between 70f73aa and cebe570.

📒 Files selected for processing (5)
  • src/main/java/com/github/_1c_syntax/bsl/languageserver/types/registry/ConfigurationModuleMembersProvider.java
  • src/main/java/com/github/_1c_syntax/bsl/languageserver/types/registry/GlobalScopeProvider.java
  • src/main/java/com/github/_1c_syntax/bsl/languageserver/types/registry/TypeRegistry.java
  • src/test/java/com/github/_1c_syntax/bsl/languageserver/types/registry/ConfigurationModuleMembersProviderTest.java
  • src/test/java/com/github/_1c_syntax/bsl/languageserver/types/registry/TypeRegistryScopedSourcesTest.java

Comment on lines +608 to +611
public void invalidateMembers(TypeRef ref) {
for (var fileType : FileType.values()) {
membersCache.remove(new MembersKey(ref, fileType));
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift

Make targeted invalidation generation-based.

Both APIs clear cached values without changing a version, allowing an in-flight stale computation to republish data after invalidation.

  • src/main/java/com/github/_1c_syntax/bsl/languageserver/types/registry/TypeRegistry.java#L608-L611: increment a canonical per-type generation and validate it in getMembers; this should also cover alternate TypeRefs resolving to the same source.
  • src/main/java/com/github/_1c_syntax/bsl/languageserver/types/registry/GlobalScopeProvider.java#L175-L176: include the GLOBAL_CONTEXT generation in GlobalIndex and reject publication from an older generation.
📍 Affects 2 files
  • src/main/java/com/github/_1c_syntax/bsl/languageserver/types/registry/TypeRegistry.java#L608-L611 (this comment)
  • src/main/java/com/github/_1c_syntax/bsl/languageserver/types/registry/GlobalScopeProvider.java#L175-L176
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@src/main/java/com/github/_1c_syntax/bsl/languageserver/types/registry/TypeRegistry.java`
around lines 608 - 611, Make targeted invalidation generation-based: in
TypeRegistry.invalidateMembers, increment a canonical per-type generation keyed
by the resolved source so alternate TypeRefs share it, and have getMembers
capture and validate that generation before publishing cached results. In
GlobalScopeProvider at the specified site, include the GLOBAL_CONTEXT generation
in GlobalIndex and reject publication from computations created under an older
generation. Apply the TypeRegistry change in TypeRegistry.java:608-611 and the
GlobalIndex change in GlobalScopeProvider.java:175-176.

@github-actions

github-actions Bot commented Jul 16, 2026

Copy link
Copy Markdown
Contributor

Test Results

 3 654 files  ± 0   3 654 suites  ±0   1h 43m 49s ⏱️ - 10m 35s
 3 637 tests + 8   3 619 ✅ + 8   18 💤 ±0  0 ❌ ±0 
21 822 runs  +48  21 710 ✅ +48  112 💤 ±0  0 ❌ ±0 

Results for commit ea8e728. ± Comparison against base commit 70f73aa.

♻️ This comment has been updated with latest results.

#4280)

По замечанию CodeRabbit: точечная инвалидация через remove не защищала от гонки —
параллельный незавершённый computeMembers мог дописать устаревший результат в кэш
уже ПОСЛЕ инвалидации (при точечной инвалидации эпоха не двигается, поэтому
устаревшая запись принималась следующим чтением).

Инвалидация теперь через пер-типовое поколение: invalidateMembers инкрементирует
поколение ключа, getMembers штампует запись поколением, снятым ДО вычисления, и
отвергает публикацию из устаревшего поколения. Аналогично GlobalScopeProvider:
name-индекс несёт поколение, invalidateNameIndex его инкрементирует, globalMember
отвергает устаревшую пересборку. Добавлен детерминированный тест гонки
(источник инициирует инвалидацию во время собственного вычисления).

Плюс метод-референсы вместо лямбд в тесте (Sonar maintainability на новом коде).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HLvkKQCwBjTHwuZ4EGBQK6

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
src/main/java/com/github/_1c_syntax/bsl/languageserver/types/registry/GlobalScopeProvider.java (1)

208-213: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win

Use compareAndSet to prevent cache thrashing.

Unconditionally setting globalIndexRef can lead to a race condition where a slow thread overwrites a fresher index (computed by another thread) with its own stale index. While this stale index will be safely rejected on the next read (because its generation won't match nameIndexGeneration), it will force an unnecessary recomputation.

Using compareAndSet ensures that we don't overwrite a newer cache entry with an older one.

⚡ Proposed optimization
     if (index == null || index.epoch() != epoch || index.generation() != generation) {
-      index = new GlobalIndex(epoch, generation, Map.of(
+      var newIndex = new GlobalIndex(epoch, generation, Map.of(
         FileType.BSL, globalNameIndex(FileType.BSL),
         FileType.OS, globalNameIndex(FileType.OS)));
-      globalIndexRef.set(index);
+      globalIndexRef.compareAndSet(index, newIndex);
+      index = newIndex;
     }
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@src/main/java/com/github/_1c_syntax/bsl/languageserver/types/registry/GlobalScopeProvider.java`
around lines 208 - 213, Update the cache refresh logic in GlobalScopeProvider to
publish the newly created GlobalIndex via globalIndexRef.compareAndSet, using
the previously observed index as the expected value, instead of unconditionally
calling set. Preserve the existing epoch and generation validation while
ensuring a slower thread cannot overwrite a newer cached index.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In
`@src/main/java/com/github/_1c_syntax/bsl/languageserver/types/registry/GlobalScopeProvider.java`:
- Around line 208-213: Update the cache refresh logic in GlobalScopeProvider to
publish the newly created GlobalIndex via globalIndexRef.compareAndSet, using
the previously observed index as the expected value, instead of unconditionally
calling set. Preserve the existing epoch and generation validation while
ensuring a slower thread cannot overwrite a newer cached index.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 0080e0c7-c7db-4722-b0d9-cb924458ee45

📥 Commits

Reviewing files that changed from the base of the PR and between cebe570 and 901aa63.

📒 Files selected for processing (4)
  • src/main/java/com/github/_1c_syntax/bsl/languageserver/types/registry/GlobalScopeProvider.java
  • src/main/java/com/github/_1c_syntax/bsl/languageserver/types/registry/TypeRegistry.java
  • src/test/java/com/github/_1c_syntax/bsl/languageserver/types/registry/ConfigurationModuleMembersProviderTest.java
  • src/test/java/com/github/_1c_syntax/bsl/languageserver/types/registry/TypeRegistryScopedSourcesTest.java
🚧 Files skipped from review as they are similar to previous changes (2)
  • src/test/java/com/github/_1c_syntax/bsl/languageserver/types/registry/ConfigurationModuleMembersProviderTest.java
  • src/main/java/com/github/_1c_syntax/bsl/languageserver/types/registry/TypeRegistry.java

…ору-источнику

Отдельный счётчик поколения name-индекса дублировал версионирование, которое уже
есть уровнем ниже: индекс — производная от getMembers(GLOBAL_CONTEXT), а тот
версионируется эпохой и пер-типовым поколением. Провайдер из-за этого делал два
вызова об одном факте (invalidateMembers(GLOBAL_CONTEXT) + invalidateNameIndex).

Теперь GlobalIndex хранит наборы-источники, из которых собран, и сверяет их
идентичность: getMembers отдаёт тот же экземпляр списка, пока memo живо, и новый —
после любой инвалидации. Этого достаточно и для актуальности, и для отбрасывания
индекса, собранного параллельно из устаревших членов, поэтому nameIndexGeneration
и invalidateNameIndex удалены вместе с лишним вызовом у провайдера.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HLvkKQCwBjTHwuZ4EGBQK6

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (2)
src/main/java/com/github/_1c_syntax/bsl/languageserver/types/registry/GlobalScopeProvider.java (2)

258-260: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low value

Pre-size the HashMap to avoid rehashing.

Since the size of the members collection is known and each member can potentially contribute two entries (Russian and English), initializing the HashMap with a sufficient initial capacity avoids runtime rehashing overhead.

⚡ Proposed fix
-  private Map<String, MemberDescriptor> globalNameIndex(Collection<MemberDescriptor> members) {
-    var map = new HashMap<String, MemberDescriptor>();
-    for (var member : members) {
+  private Map<String, MemberDescriptor> globalNameIndex(Collection<MemberDescriptor> members) {
+    // Both RU and EN names might be added, so size the map to avoid rehashing
+    var map = new HashMap<String, MemberDescriptor>(members.size() * 2);
+    for (var member : members) {
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@src/main/java/com/github/_1c_syntax/bsl/languageserver/types/registry/GlobalScopeProvider.java`
around lines 258 - 260, Update globalNameIndex to initialize its HashMap with an
initial capacity based on members.size() multiplied by two, accounting for
possible Russian and English entries and avoiding rehashing.

181-194: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win

Use compareAndSet to prevent stale cache overwrites and update outdated Javadoc.

If two threads concurrently update the index after an invalidation, a thread with an older bslSource/osSource can overwrite a newer cached index using set, causing an unnecessary cache miss on the next call. Using compareAndSet prevents this race condition.

Additionally, please update the method's Javadoc (around line 169). It still references пересобираемому при смене эпохи членов ({@link TypeRegistry#membersEpoch()}), which contradicts the new identity-based caching logic. As per coding guidelines, keep documentation up to date with code changes.

⚡ Proposed fix
-    if (index == null || index.bslSource() != bslSource || index.osSource() != osSource) {
-      index = new GlobalIndex(bslSource, osSource, Map.of(
-        FileType.BSL, globalNameIndex(bslSource),
-        FileType.OS, globalNameIndex(osSource)));
-      globalIndexRef.set(index);
-    }
+    if (index == null || index.bslSource() != bslSource || index.osSource() != osSource) {
+      var newIndex = new GlobalIndex(bslSource, osSource, Map.of(
+        FileType.BSL, globalNameIndex(bslSource),
+        FileType.OS, globalNameIndex(osSource)));
+      globalIndexRef.compareAndSet(index, newIndex);
+      index = newIndex;
+    }
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@src/main/java/com/github/_1c_syntax/bsl/languageserver/types/registry/GlobalScopeProvider.java`
around lines 181 - 194, Update the global index cache logic in the relevant
provider method to publish newly built GlobalIndex instances with
globalIndexRef.compareAndSet(index, newIndex) instead of unconditional set,
preserving the existing cached value when another thread has already installed a
newer index. Also revise that method’s Javadoc to remove the membersEpoch-based
rebuild description and document the current source-identity caching behavior.

Source: Coding guidelines

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In
`@src/main/java/com/github/_1c_syntax/bsl/languageserver/types/registry/GlobalScopeProvider.java`:
- Around line 258-260: Update globalNameIndex to initialize its HashMap with an
initial capacity based on members.size() multiplied by two, accounting for
possible Russian and English entries and avoiding rehashing.
- Around line 181-194: Update the global index cache logic in the relevant
provider method to publish newly built GlobalIndex instances with
globalIndexRef.compareAndSet(index, newIndex) instead of unconditional set,
preserving the existing cached value when another thread has already installed a
newer index. Also revise that method’s Javadoc to remove the membersEpoch-based
rebuild description and document the current source-identity caching behavior.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: efae6277-8537-4700-8611-7278195969ac

📥 Commits

Reviewing files that changed from the base of the PR and between 901aa63 and 880d75d.

📒 Files selected for processing (2)
  • src/main/java/com/github/_1c_syntax/bsl/languageserver/types/registry/ConfigurationModuleMembersProvider.java
  • src/main/java/com/github/_1c_syntax/bsl/languageserver/types/registry/GlobalScopeProvider.java
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/main/java/com/github/_1c_syntax/bsl/languageserver/types/registry/ConfigurationModuleMembersProvider.java

nixel2007 and others added 3 commits July 21, 2026 18:40
Дерево зависимостей проекта даёт в jmh-архиве больше 65535 записей, поэтому
задача jmhJar падала с «Archive contains more than 65535 entries» — запустить
./gradlew jmh было нельзя вообще. Включаем zip64 у архива.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HLvkKQCwBjTHwuZ4EGBQK6
…ления

Полный прогон анализа на cpm не разрешает накладной расход проверки поколения:
разброс baseline между прогонами доходит до 1м41с, а проверка добавляет один
поиск в хеш-карте на вызов. Микробенчмарк меряет это напрямую.

realGetMembersHit — реальный TypeRegistry (440 типов, memo прогрето) на попадании.
lookupEpochOnly и lookupEpochAndGeneration — изолированные модели проверки
актуальности «до» и «после» на картах того же размера; их разница и есть цена
добавленного поиска. Параметр invalidatedTypes задаёт размер карты поколений:
0 — правок не было, 440 — инвалидировались все типы рабочей области.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HLvkKQCwBjTHwuZ4EGBQK6
…алидации

- globalMember публикует пересобранный индекс через compareAndSet: параллельный
  поток мог опубликовать индекс по более свежим наборам, и затирать его своим не
  нужно — иначе следующее чтение зря пересоберёт индекс;
- javadoc globalMember больше не ссылается на эпоху членов как на критерий
  пересборки: после перехода на сверку наборов-источников это стало неверно;
- globalNameIndex сразу задаёт ёмкость карты (до двух записей на член, ru и en);
- тест инвалидации проверяет, что memo отредактированного модуля именно
  пересобрано (новый экземпляр списка): проверка только по содержимому проходила
  бы и в случае, когда инвалидации не произошло вовсе.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HLvkKQCwBjTHwuZ4EGBQK6
@nixel2007

Copy link
Copy Markdown
Member Author

@coderabbitai учтено, спасибо — разобрал и actionable, и все nit'ы.

Major (generation-based invalidation). Точечная инвалидация переведена на пер-типовое поколение (901aa63): invalidateMembers инкрементирует поколение ключа вместо remove, getMembers штампует запись поколением, снятым до вычисления, и отвергает публикацию из устаревшего — параллельный незавершённый computeMembers, дописывающий результат уже после инвалидации, больше не «переживает» её. Добавлен детерминированный тест гонки staleInFlightComputeIsRejectedAfterInvalidation (источник инициирует инвалидацию во время собственного вычисления; на старом remove-коде тест красный).

Про «alternate TypeRefs resolving to the same source»: invalidateMembers вызывается только с каноническими интернированными ссылками — конфигурационные типы интернируются один раз, а ru/en-имена ведут на ту же ссылку через aliasIndex, поэтому отдельных alias-TypeRef, которые могли бы промахнуться, нет.

Про поколение name-индекса — оно в итоге убрано совсем (880d75d). Отдельный счётчик дублировал версионирование, которое уже есть уровнем ниже: индекс — производная от getMembers(GLOBAL_CONTEXT), а тот версионируется эпохой и пер-типовым поколением. Теперь GlobalIndex хранит наборы-источники и сверяет их идентичность (getMembers отдаёт тот же экземпляр списка, пока memo живо, и новый — после любой инвалидации), так что nameIndexGeneration и invalidateNameIndex удалены вместе с лишним вызовом у провайдера.

Nit'ы (6d1e8f5):

  • compareAndSet вместо set при публикации индекса — да, верно: незачем затирать более свежий индекс своим и провоцировать лишнюю пересборку;
  • javadoc globalMember действительно устарел после перехода на сверку наборов — ссылка на эпоху как критерий пересборки убрана;
  • ёмкость HashMap в globalNameIndex задаётся сразу (до двух записей на член, ru/en);
  • усиление теста инвалидации — принято и оказалось по делу: проверка только по содержимому проходила бы и в случае, когда инвалидации не произошло вовсе. Теперь тест требует новый экземпляр списка (isNotSameAs) и в таком виде проходит.

По цене guard'а (на случай вопроса, стоил ли он того): микробенчмарк JMH MembersCacheLookup — проверка поколения добавляет ~1.9 ns/оп на пустой карте поколений и ~3.4 ns на заполненной; реальный getMembers на попадании — 5.7/8.4 ns. На масштабе прогона это десятки миллисекунд против ~60 с, которые экономит сам фикс.

@coderabbitai

coderabbitai Bot commented Jul 21, 2026

Copy link
Copy Markdown
Contributor

Tip

For best results, initiate chat on the files or code changes.

@nixel2007 Спасибо за подробное разъяснение и исправления. Пер-типовые поколения вместе с детерминированным тестом гонки закрывают критичный сценарий устаревшей публикации, а сравнение identity наборов-источников делает инвалидацию GlobalIndex согласованной с мемоизацией getMembers. Бенчмарк также убедительно подтверждает, что стоимость guard-а оправдана выигрышем пакетного анализа.

🐇✨


✏️ Learnings added
Learnt from: nixel2007
URL: https://github.com/1c-syntax/bsl-language-server/pull/4280

Timestamp: 2026-07-21T17:13:35.637Z
Learning: In the Java type registry, configuration types use canonical interned `TypeRef` instances: Russian and English names resolve through `aliasIndex` to the same reference. Therefore, `TypeRegistry.invalidateMembers(TypeRef)` is called with canonical references and does not need to invalidate separate alias `TypeRef` objects.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.


If you found this review helpful, would you consider giving us a shout-out on X?

Thank you for using CodeRabbit!

…й эпохи

BSL перевели на точечную инвалидацию раньше, а .os остался на широком сдвиге
эпохи — то есть в OneScript-рабочей области фикса не было вообще: каждый
rebuildDocument на пакетном анализе по-прежнему обесценивал memo всех типов.
Вдобавок эпоха одна на реестр, поэтому правка .os сбрасывала и кэши BSL-типов,
хотя источники членов строго разделены по языку (resolveMemberSources читает
memberSources.get(fileType)) и пересекаться не могут.

Теперь OScriptModuleMembersProvider на изменение содержимого сбрасывает memo
правленого документа и его наследников, обходя TypeRelations.subtypes вширь.
Обход обязан быть транзитивным: inheritedMembers копирует к себе результат
getMembers родителя, поэтому в memo наследника лежит снимок членов родителя —
сброса одного родителя мало, снимок протухает на всю глубину иерархии.
Реализаторов интерфейсов обходить не нужно: &Реализует членов не приносит,
inheritedMembers резолвит только родительский класс. Повторные посещения
отсекаются по URI, это же защищает от циклов в объявлениях наследования.

Листенер TypeRegistry.invalidateMembersCache удалён целиком — после перевода
обоих языков на точечную инвалидацию ему нечего делать. Вместе с ним ушёл
последний @eventlistener реестра (оба импорта), а публичный membersEpoch()
остался без потребителей после перехода name-индекса на сверку наборов-
источников и удалён как мёртвый. membersEpoch теперь версионирует ровно
структурные мутации реестра.

Тест editingBaseClassRebuildsMembersOfGrandchildTransitively на фикстуре
extends-lib (База ← Промежуточный ← Дочерний) правит базу и требует пересборки
memo внука; проверен на красноту — без обхода наследников падает.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HLvkKQCwBjTHwuZ4EGBQK6
@sonarqubecloud

Copy link
Copy Markdown

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant