perf(context): кэшировать разбор doc-комментариев символов#4175
Open
nixel2007 wants to merge 2 commits into
Open
perf(context): кэшировать разбор doc-комментариев символов#4175nixel2007 wants to merge 2 commits into
nixel2007 wants to merge 2 commits into
Conversation
Contributor
|
Note Reviews pausedIt 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 Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
🚧 Files skipped from review as they are similar to previous changes (5)
📝 WalkthroughWalkthroughIntroduces ChangesSymbol Description Index
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/main/java/com/github/_1c_syntax/bsl/languageserver/context/computer/SymbolDescriptionCache.java (1)
70-72: Replace manual private constructor with@UtilityClassannotation.This class is a pure static utility with no instance members. Replace the manual private constructor pattern with Lombok's
@UtilityClassannotation to align with the repository's established coding standard and reduce boilerplate code.Import
lombok.experimental.UtilityClassand annotate the class:Suggested change
import lombok.experimental.UtilityClass; `@UtilityClass` final class SymbolDescriptionCache { // remove private constructor // ...rest of class remains unchanged }🤖 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/context/computer/SymbolDescriptionCache.java` around lines 70 - 72, The SymbolDescriptionCache class uses a manual private constructor pattern for a pure static utility class. Replace this boilerplate by adding the import statement for lombok.experimental.UtilityClass at the top of the file, annotating the SymbolDescriptionCache class with `@UtilityClass`, making the class final, and removing the private SymbolDescriptionCache() constructor method entirely. This aligns with the repository's coding standards and Lombok provides the same behavior automatically.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/context/computer/SymbolDescriptionCache.java`:
- Around line 70-72: The SymbolDescriptionCache class uses a manual private
constructor pattern for a pure static utility class. Replace this boilerplate by
adding the import statement for lombok.experimental.UtilityClass at the top of
the file, annotating the SymbolDescriptionCache class with `@UtilityClass`, making
the class final, and removing the private SymbolDescriptionCache() constructor
method entirely. This aligns with the repository's coding standards and Lombok
provides the same behavior automatically.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 83dfa483-2e6e-48fa-bf24-ed1066425676
📒 Files selected for processing (3)
src/main/java/com/github/_1c_syntax/bsl/languageserver/context/computer/MethodSymbolComputer.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/context/computer/SymbolDescriptionCache.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/context/computer/VariableSymbolComputer.java
nixel2007
force-pushed
the
claude/perf-description-cache
branch
from
June 20, 2026 07:25
5c1bb42 to
e93b70e
Compare
Contributor
nixel2007
force-pushed
the
claude/perf-description-cache
branch
from
June 20, 2026 09:26
e93b70e to
251d168
Compare
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/test/java/com/github/_1c_syntax/bsl/languageserver/context/computer/SymbolDescriptionIndexTest.java (1)
81-89: ⚡ Quick winStrengthen variable-path assertions to validate cache semantics.
This block only checks non-null results, so a regression in opened-doc caching or not-opened bypass would still pass. Assert identity behavior here as you already do for methods.
Suggested test tightening
- var openedResult = index.variableDescription(opened, opened.getComments(), Optional.empty()); - assertThat(openedResult).as("описание переменной открытого документа получено").isNotNull(); - assertThat(index.variableDescription(opened, opened.getComments(), Optional.empty())) - .as("повторный вызов открытого документа не падает").isNotNull(); + var openedFirst = index.variableDescription(opened, opened.getComments(), Optional.empty()); + var openedSecond = index.variableDescription(opened, opened.getComments(), Optional.empty()); + assertThat(openedFirst).as("описание переменной открытого документа получено").isNotNull(); + assertThat(openedSecond).as("у открытого документа описание переменной берётся из кэша").isSameAs(openedFirst); // Не-открытый документ — прямой разбор мимо кэша. var notOpened = TestUtils.getDocumentContext(varSource); - assertThat(index.variableDescription(notOpened, notOpened.getComments(), Optional.empty())) - .as("описание переменной не-открытого документа получено напрямую").isNotNull(); + var notOpenedFirst = index.variableDescription(notOpened, notOpened.getComments(), Optional.empty()); + var notOpenedSecond = index.variableDescription(notOpened, notOpened.getComments(), Optional.empty()); + assertThat(notOpenedFirst).as("описание переменной не-открытого документа получено напрямую").isNotNull(); + assertThat(notOpenedSecond).as("не-открытый документ не должен кешировать описание переменной") + .isNotSameAs(notOpenedFirst);🤖 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/context/computer/SymbolDescriptionIndexTest.java` around lines 81 - 89, The test currently only validates that the variableDescription method returns non-null results but does not verify the caching behavior for opened documents or the direct bypass for not-opened documents. Strengthen the assertions by checking object identity: for the opened document, verify that calling variableDescription twice on the same opened instance returns the identical object (using identity assertion as already done for methods in this test), demonstrating caching is working; the notOpened document assertion can remain as-is since it's testing the direct bypass path. This will catch regressions where caching logic or the not-opened bypass might be broken while still returning non-null values.
🤖 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/test/java/com/github/_1c_syntax/bsl/languageserver/context/computer/SymbolDescriptionIndexTest.java`:
- Around line 81-89: The test currently only validates that the
variableDescription method returns non-null results but does not verify the
caching behavior for opened documents or the direct bypass for not-opened
documents. Strengthen the assertions by checking object identity: for the opened
document, verify that calling variableDescription twice on the same opened
instance returns the identical object (using identity assertion as already done
for methods in this test), demonstrating caching is working; the notOpened
document assertion can remain as-is since it's testing the direct bypass path.
This will catch regressions where caching logic or the not-opened bypass might
be broken while still returning non-null values.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 96f42e5f-44e4-4d70-82b8-0ce6ce85f994
📒 Files selected for processing (5)
src/main/java/com/github/_1c_syntax/bsl/languageserver/context/DocumentContext.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/context/computer/MethodSymbolComputer.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/context/computer/SymbolDescriptionIndex.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/context/computer/VariableSymbolComputer.javasrc/test/java/com/github/_1c_syntax/bsl/languageserver/context/computer/SymbolDescriptionIndexTest.java
🚧 Files skipped from review as they are similar to previous changes (2)
- src/main/java/com/github/_1c_syntax/bsl/languageserver/context/computer/MethodSymbolComputer.java
- src/main/java/com/github/_1c_syntax/bsl/languageserver/context/computer/VariableSymbolComputer.java
nixel2007
force-pushed
the
claude/perf-description-cache
branch
from
June 20, 2026 09:36
251d168 to
e2829a5
Compare
Symbol tree открытого документа перестраивается на каждый keystroke, и при сборке для каждого метода/переменной жадно вызывался MethodDescription.create / VariableDescription.create — полный отдельный ANTLR-разбор грамматики комментария (по профилю набора текста — заметная доля стоимости перестроения), хотя текст комментариев между нажатиями почти не меняется. SymbolDescriptionIndex (@WorkspaceScope) хранит разобранные описания по URI и отдаёт их повторно. Ключевые свойства: - кэшируется только для ОТКРЫТЫХ в редакторе документов (isDocumentOpened); для не-открытых (populateContext, пакетный анализ из CLI / sonar-bsl-community- plugin) документ разбирается один раз — кэш бесполезен, поэтому путь идёт мимо него (прямой create), без затрат памяти; - записи привязаны к URI и сбрасываются на закрытии/освобождении/удалении документа (Closed/Cleared/Removed); на изменение содержимого индекс НЕ сбрасывается (иначе терялся бы смысл), устаревшие записи вытесняются по размеру; - ключ контент-адресный (line:charPositionInLine:text по токенам) — захватывает и текст (результат разбора), и позиции (SimpleRange описания, которые читают семантические токены и диагностики), поэтому попадание всегда корректно и по содержимому, и по диапазонам. Замер: попадание в кэш ~48–51x быстрее разбора и ~14–26x меньше аллокаций; на keystroke-rebuild документированного модуля computeSymbolTree по сэмплам ~−48%. Память: на populate/batch — 0; на один открытый модуль (УправлениеДоступом, 3733 строки, 61 описание) — ~0.14 МБ ключей (сами описания общие с symbol tree), освобождается на закрытии документа. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HcK3qVwTH91rXtgprGmoWr
nixel2007
force-pushed
the
claude/perf-description-cache
branch
from
June 20, 2026 09:38
e2829a5 to
d334cb7
Compare
|
Правки, сдвигающие номера строк, на каждый ребилд плодят новое поколение позиционных ключей; на крупных модулях (замер на УправлениеДоступомСлужебный, 48k строк: 7279 записей при потолке 8192 уже после нескольких блоков набора) пер-документный кэш быстро упирается в maximumSize, удерживая устаревшие описания. Добавлен expireAfterAccess(1 мин) к обоим кэшам: после паузы в наборе устаревшие ключи отваливаются, не дожидаясь вытеснения по размеру. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HcK3qVwTH91rXtgprGmoWr
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Зачем
Symbol tree перестраивается на каждый keystroke, и при сборке для каждого метода/переменной жадно вызывался
MethodDescription.create(...)/VariableDescription.create(...)— это полный отдельный ANTLR-разбор грамматики doc-комментария (свои лексер + парсер + обход на каждый комментарий). Текст комментариев между нажатиями почти не меняется, поэтому этот разбор переделывался впустую.По профилю набора текста (SSL 3.2, модуль
УправлениеДоступом, 3733 строки, плотно документирован) видимая self-time кадровBSLDescriptionParser.*была ~9%, но их «подводная» часть — ANTLR-машинерия, которую они запускают, — учитывалась под общими ANTLR-кадрами. Суммарно разбор описаний составлял почти половину стоимостиcomputeSymbolTree.Что меняется
SymbolDescriptionCache— контент-адресный кэш (Caffeine, bounded 50k) разобранных описаний. Точки вызова вMethodSymbolComputer/VariableSymbolComputerидут через кэш вместо прямого…Description.create(...).Ключ — сигнатуры токенов комментария
line:charPositionInLine:text. Это намеренно захватывает:SimpleRangeвнутри описания, которые читают семантические токены (BslDocSemanticTokens/CommentSemanticTokens) и диагностики (MissingParameterDescription,LineLength).Поэтому попадание в кэш всегда возвращает описание, корректное и по содержимому, и по диапазонам. Описания неизменяемы и являются чистой функцией входных токенов → переиспользование экземпляра (в т.ч. между документами и потоками параллельного
populateContext) безопасно.Правки внутри строки (большинство нажатий) не сдвигают номера строк → позиции и текст комментариев прежние → попадание. Вставка/удаление строки сдвигает позиции ниже правки → промах и корректный пере-разбор уже с новыми диапазонами.
Замеры
Эмуляция набора текста на
УправлениеДоступом(JFR на keystroke-rebuild), best-case по hit-rate (правки в конце модуля / внутри строки, позиции комментариев стабильны):BSLDescriptionParser.*полностью уходит из топа hotspot'ов. Выигрыш масштабируется с плотностью doc-комментариев (SSL документирован плотно → крупный эффект; для слабо документированных модулей — меньше).Проверка
compileJavaзелёный.MethodSymbolComputerTest,MethodSymbolComputerConstructorTest,VariableSymbolComputer*,SymbolTreeComputerTest,BslDocSemanticTokensSupplierTest,CommentSemanticTokensSupplierTest,MissingParameterDescriptionDiagnosticTest,HoverProviderTest,HoverProviderTypeAwareTest,SignatureHelpProviderTest,SignatureHelpCommonModuleTest.Изменение независимо от #4174 (другие файлы), базируется на
develop.🤖 Generated with Claude Code
https://claude.ai/code/session_01HcK3qVwTH91rXtgprGmoWr
Generated by Claude Code
Summary by CodeRabbit