perf(diagnostics): ограничение пула JLanguageTool#4260
Conversation
JLanguageToolPool рос неограниченно — до числа потоков анализа: при parallelStream на 24-ядерной машине создавалось ~20 инстансов JLanguageTool, каждый при создании загружает собственную копию полного набора правил языка (~35 МБ на инстанс; последующий disableRule память не освобождает). На analyze УПП это давало ~680 МБ live heap только под инфраструктуру диагностики Typo. AbstractObjectPool получил опциональный maxSize: при исчерпании лимита checkOut() блокируется до возврата объекта через checkIn() (wait/notify; прерывание потока не срывает выдачу — флаг прерывания восстанавливается). JLanguageToolPool ограничен min(4, доступных ядер). Конкуренция за пул низкая: статусы слов кэшируются в CheckedWordsHolder, к LT уходят только ещё не проверенные слова. Замер (analyze УПП, холодный кэш слов, jcmd GC.class_histogram, live): JLanguageTool 20 → 4 инстансов, прямые объекты LanguageTool 359 → 80 МБ (~-400 МБ с учётом шаренных структур); диагностики эквивалентны (SARIF с идентичными счётчиками правил), время анализа не выросло (2:34 → 2:21). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
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 (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthrough
ChangesBounded object pooling
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant CheckoutThread
participant AbstractObjectPool
participant ReturningThread
CheckoutThread->>AbstractObjectPool: checkOut()
AbstractObjectPool-->>CheckoutThread: wait at maxSize
ReturningThread->>AbstractObjectPool: checkIn(instance)
AbstractObjectPool-->>CheckoutThread: notifyAll()
AbstractObjectPool-->>CheckoutThread: return available or newly created object
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/main/java/com/github/_1c_syntax/bsl/languageserver/utils/AbstractObjectPool.java (1)
74-76: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winGuard against non-positive
maxSizein the bounded constructor.If a subclass passes
maxSize <= 0,checkOut()enters the wait loop (available.isEmpty() && inUse.size() >= maxSizeistruewith an empty pool),create()is never reached, and nothing can evercheckIn()— the caller hangs forever with no diagnostic. The only current caller is safe, but this is aprotectedbase-class API, so a cheap precondition prevents a silent deadlock for future subclasses.🛡️ Proposed guard
protected AbstractObjectPool(int maxSize) { + if (maxSize < 1) { + throw new IllegalArgumentException("maxSize must be >= 1, got: " + maxSize); + } this.maxSize = maxSize; }🤖 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/utils/AbstractObjectPool.java` around lines 74 - 76, Add a constructor precondition in AbstractObjectPool(int maxSize) that rejects non-positive values, throwing an appropriate argument-validation exception with a clear diagnostic before assigning maxSize; preserve valid positive sizes and leave unbounded construction behavior unchanged.
🤖 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/utils/AbstractObjectPool.java`:
- Around line 74-76: Add a constructor precondition in AbstractObjectPool(int
maxSize) that rejects non-positive values, throwing an appropriate
argument-validation exception with a clear diagnostic before assigning maxSize;
preserve valid positive sizes and leave unbounded construction behavior
unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: a136dc9b-af93-432d-918e-0d0c0c17c08a
📒 Files selected for processing (2)
src/main/java/com/github/_1c_syntax/bsl/languageserver/diagnostics/typo/JLanguageToolPool.javasrc/main/java/com/github/_1c_syntax/bsl/languageserver/utils/AbstractObjectPool.java
java:S2142 — флаг прерывания восстанавливается сразу в catch; прерванный ожидающий поток выходит из ожидания и получает объект сверх лимита (временное превышение maxSize на число прерванных ожидающих), вместо отложенного восстановления флага после цикла ожидания. java:S2446 — notifyAll() вместо notify(). Юнит-тесты AbstractObjectPool: переиспользование возвращённого экземпляра, рост неограниченного пула, блокировка на лимите с передачей экземпляра, поведение при прерывании, toString. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
| public synchronized void checkIn(T instance) { | ||
| inUse.remove(instance); | ||
| available.add(instance); | ||
| notifyAll(); |
There was a problem hiding this comment.
До ограничения пула checkOut() никогда не ждал: при пустом available сразу создавался новый экземпляр. С появлением maxSize поток при исчерпании лимита встаёт в wait() — без notify* в checkIn() он не проснётся никогда.
notifyAll вместо notify — по замечанию Sonar (java:S2446): notify будит произвольный поток. Ожидающие здесь однородны, так что технически хватило бы и notify, но notifyAll — стандартный безопасный паттерн, а лишних пробуждений при размере пула ≤4 единицы.
По замечанию ревью: maxSize < 1 приводил бы к вечному ожиданию в checkOut без диагностики — теперь конструктор бросает IllegalArgumentException. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
2 participants
Суть
JLanguageToolPoolрос неограниченно — до числа потоков анализа: приparallelStreamна 24-ядерной машине создавалось ~20 инстансовJLanguageTool, каждый при создании загружает собственную копию полного набора правил языка (~35 МБ на инстанс; последующийdisableRuleпамять не освобождает). НаanalyzeУПП это давало ~680 МБ live heap только под инфраструктуру диагностики Typo.AbstractObjectPoolполучил опциональныйmaxSize: при исчерпании лимитаcheckOut()блокируется до возврата объекта черезcheckIn()(wait/notify; прерывание потока не срывает выдачу — флаг прерывания восстанавливается перед возвратом).JLanguageToolPoolограниченmin(4, доступных ядер). Конкуренция за пул низкая: статусы слов кэшируются вCheckedWordsHolder(персистентный EhCache), к LT уходят только ещё не проверенные слова.Замер
analyzeУПП (~9 500 модулей), холодный кэш слов,jcmd GC.class_histogram(live-объекты):JLanguageToolDisambiguationPatternRuleС учётом шаренных структур (CHM/массивы/строки словарей) экономия — примерно −400 МБ на 24-ядерной машине (на машинах с меньшим числом ядер эффект меньше — пул и раньше не вырастал большим). Диагностики эквивалентны (SARIF с идентичными счётчиками правил), время анализа не выросло (2:34 → 2:21).
Проверки
./gradlew test: 3 582 теста, 0 упавших;🤖 Generated with Claude Code
Summary by CodeRabbit
Performance
Reliability
Tests