Skip to content

perf(reader): lock-free ConcurrentQNameMap (снятие контеншена на synchronizedMap)#645

Merged
nixel2007 merged 2 commits into
developfrom
perf/concurrent-qnamemap
Jul 15, 2026
Merged

perf(reader): lock-free ConcurrentQNameMap (снятие контеншена на synchronizedMap)#645
nixel2007 merged 2 commits into
developfrom
perf/concurrent-qnamemap

Conversation

@nixel2007

@nixel2007 nixel2007 commented Jul 15, 2026

Copy link
Copy Markdown
Member

Описание

Профилирование чтения (JFR на cpm) показало, что QNameMap.getJavaClassName вызывается на
каждый XML-элемент и делает get по Collections.synchronizedMap. Метаданные читаются
параллельно (parallelStream), поэтому все потоки сериализуются на мониторе одной общей карты.

Вводим ConcurrentQNameMap — наследник QNameMap с картами на ConcurrentHashMap (чтение без
блокировок). Отображения регистрируются один раз до начала чтения, во время чтения карты только
читаются. Поведение не меняется.

Замеры (JMH MDClassesBenchmark на cpm; CPU — доля JFR-сэмплов, устойчива к шуму машины)

Правильный объект профилирования — стоимость самой операции поиска getJavaClassName
(inclusive)
, а не фрейм реализации: в новой версии synchronizedMap физически нет, поэтому
считать его сэмплы бессмысленно. Inclusive-метрика учитывает и новую стоимость ConcurrentHashMap.get:

метрика before (develop) after
getJavaClassName (inclusive) в CPU чтения 332/5609 = 5.9 % 83/8927 = 0.9 % (−5 п.п.)
аллокации 5154.6 МБ/чтение 5157.9 МБ/чтение (без изменений)

Т.е. вся операция поиска (включая ConcurrentHashMap.get в новой версии) подешевела ~5 п.п. —
за счёт снятия монитора/контеншена на общей synchronizedMap. Wall на тестовой машине имеет
разброс ~±35 %, поэтому по нему изменение ~5 % не разрешается; CPU меряем долей сэмплов.

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

Closes

Чеклист

Общие

  • Ветка PR обновлена из develop
  • Отладочные, закомментированные и прочие, не имеющие смысла участки кода удалены
  • Изменения покрыты тестами (ConcurrentQNameMapTest; интеграционные MDOReaderTest/MDMergerMergeTest читают Designer-формы через это отображение)
  • Обязательные действия перед коммитом выполнены (gradlew precommit) — прогонял reader-тесты

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

  • registerMapping помечен synchronized под контракт родителя (Sonar S3551); вызывается только
    на этапе настройки, на горячий путь не влияет.
  • Метрика исправлена по замечанию ревью: считаем getJavaClassName inclusive, а не сам
    SynchronizedMap.get (которого в новой версии нет).

…ap contention

Профилирование чтения показало, что QNameMap.getJavaClassName вызывается на каждый
XML-элемент и делает get по Collections.synchronizedMap. Метаданные читаются
параллельно (parallelStream), поэтому все потоки сериализуются на мониторе одной
общей карты — по профилю ~5% CPU уходит в SynchronizedMap.get.

Вводим ConcurrentQNameMap с картами на ConcurrentHashMap (чтение без блокировок).
Отображения регистрируются один раз до чтения, во время чтения карты только читаются.

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 15, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

Adds a thread-safe QNameMap implementation backed by concurrent caches and configures DesignerReader to use it for XML mapping.

Changes

Concurrent QName mapping

Layer / File(s) Summary
Concurrent QNameMap implementation
src/main/java/com/github/_1c_syntax/bsl/reader/common/xstream/ConcurrentQNameMap.java
Adds concurrent QName-to-class and class-to-QName caches, lookup fallbacks, and mapping registration.
Designer XML mapper integration
src/main/java/com/github/_1c_syntax/bsl/reader/designer/DesignerReader.java
Replaces XStream’s QNameMap with ConcurrentQNameMap when creating the XML mapper.

Estimated code review effort: 2 (Simple) | ~10 minutes

Suggested reviewers: theshadowco

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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 The title clearly matches the main change: replacing the reader's QName map with a lock-free ConcurrentQNameMap to reduce synchronizedMap contention.
✨ 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/concurrent-qnamemap

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.

@github-actions

github-actions Bot commented Jul 15, 2026

Copy link
Copy Markdown
Contributor

Test Results

  522 files  + 6    522 suites  +6   12m 38s ⏱️ +37s
1 259 tests + 3  1 256 ✅ + 3   3 💤 ±0  0 ❌ ±0 
7 578 runs  +18  7 560 ✅ +18  18 💤 ±0  0 ❌ ±0 

Results for commit 1fef2fa. ± Comparison against base commit 5f0aafd.

♻️ This comment has been updated with latest results.

- registerMapping помечен synchronized, чтобы соответствовать контракту родителя
  QNameMap (Sonar S3551); метод вызывается только на этапе настройки, на горячий
  путь чтения не влияет.
- Добавлен ConcurrentQNameMapTest (регистрация/резолв, фолбэки) — покрытие нового
  класса.

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

@nixel2007
nixel2007 merged commit 48f6b6e into develop Jul 15, 2026
19 checks passed
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