Бенчмарк#528
Conversation
WalkthroughДобавлены JMH-бенчмарки и InternalProfiler для сбора памяти/GC, интеграция JMH в Gradle, скрипт для сравнения бенчмарков (ветка/ JAR), и Python-утилита для анализа/визуализации результатов. Расширен .gitignore для каталога benchmark-results. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Dev as User/CI
participant Sh as benchmark-compare.sh
participant Git as Git
participant Gradle as Gradle (jmhJar)
participant JMH as java -jar (JMH Runner)
participant Prof as MemoryProfiler
participant FS as FS (benchmark-results)
participant Py as benchmark-analyze-results.py
Dev->>Sh: Запуск с параметрами old/new (ветка или JAR)
alt Источник = ветка
Sh->>Git: checkout <branch>
Sh->>Gradle: gradlew clean jmhJar
Gradle-->>Sh: jmh JAR
else Источник = JAR
Sh->>FS: Копирование JAR в результаты
end
Sh->>JMH: java -jar old-version.jar
JMH->>Prof: beforeIteration()
Prof->>JMH: собирает/возвращает метрики (после итерации)
JMH-->>FS: old-results.json
Sh->>JMH: java -jar new-version.jar
JMH->>Prof: beforeIteration()
Prof->>JMH: собирает/возвращает метрики
JMH-->>FS: new-results.json
Sh->>Py: запускает анализ `benchmark-analyze-results.py`
Py-->>FS: PNG-графики, отчёт
Sh-->>Dev: Путь к результатам и сводка
sequenceDiagram
autonumber
participant JMH as JMH Runner
participant BM as MDClassesBenchmark
participant Prof as MemoryProfiler
participant MX as JVM MXBeans / GC
JMH->>BM: @Setup(Level.Trial)
loop Каждая итерация
JMH->>Prof: beforeIteration()
Prof->>MX: зафиксировать метрики, вызвать System.gc()
JMH->>BM: выполнить @Benchmark
BM->>BM: MDClasses.createConfiguration(...)
JMH->>Prof: afterIteration()
Prof->>MX: снять дельты памяти/GC
Prof-->>JMH: вернуть ScalarResult (memory*, gc.*)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60–90 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 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.
Actionable comments posted: 6
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
.gitignore(1 hunks)benchmark-analyze-results.py(1 hunks)benchmark-compare.sh(1 hunks)build.gradle.kts(4 hunks)src/jmh/java/com/github/_1c_syntax/bsl/mdclasses/benchmark/MDClassesBenchmark.java(1 hunks)src/jmh/java/com/github/_1c_syntax/bsl/mdclasses/benchmark/MemoryProfiler.java(1 hunks)src/jmh/java/com/github/_1c_syntax/bsl/mdclasses/benchmark/package-info.java(1 hunks)
🧰 Additional context used
🪛 Ruff (0.13.3)
benchmark-analyze-results.py
8-8: Docstring contains ambiguous с (CYRILLIC SMALL LETTER ES). Did you mean c (LATIN SMALL LETTER C)?
(RUF002)
13-13: String contains ambiguous С (CYRILLIC CAPITAL LETTER ES). Did you mean C (LATIN CAPITAL LETTER C)?
(RUF001)
56-56: Comment contains ambiguous В (CYRILLIC CAPITAL LETTER VE). Did you mean B (LATIN CAPITAL LETTER B)?
(RUF003)
56-56: Comment contains ambiguous С (CYRILLIC CAPITAL LETTER ES). Did you mean C (LATIN CAPITAL LETTER C)?
(RUF003)
56-56: Comment contains ambiguous Е (CYRILLIC CAPITAL LETTER IE). Did you mean E (LATIN CAPITAL LETTER E)?
(RUF003)
68-68: String contains ambiguous В (CYRILLIC CAPITAL LETTER VE). Did you mean B (LATIN CAPITAL LETTER B)?
(RUF001)
81-81: String contains ambiguous В (CYRILLIC CAPITAL LETTER VE). Did you mean B (LATIN CAPITAL LETTER B)?
(RUF001)
83-83: String contains ambiguous Н (CYRILLIC CAPITAL LETTER EN). Did you mean H (LATIN CAPITAL LETTER H)?
(RUF001)
83-83: String contains ambiguous Е (CYRILLIC CAPITAL LETTER IE). Did you mean E (LATIN CAPITAL LETTER E)?
(RUF001)
85-85: f-string without any placeholders
Remove extraneous f prefix
(F541)
92-92: f-string without any placeholders
Remove extraneous f prefix
(F541)
92-92: String contains ambiguous В (CYRILLIC CAPITAL LETTER VE). Did you mean B (LATIN CAPITAL LETTER B)?
(RUF001)
92-92: String contains ambiguous С (CYRILLIC CAPITAL LETTER ES). Did you mean C (LATIN CAPITAL LETTER C)?
(RUF001)
92-92: String contains ambiguous Е (CYRILLIC CAPITAL LETTER IE). Did you mean E (LATIN CAPITAL LETTER E)?
(RUF001)
140-140: Docstring contains ambiguous с (CYRILLIC SMALL LETTER ES). Did you mean c (LATIN SMALL LETTER C)?
(RUF002)
193-193: Comment contains ambiguous с (CYRILLIC SMALL LETTER ES). Did you mean c (LATIN SMALL LETTER C)?
(RUF003)
348-348: String contains ambiguous В (CYRILLIC CAPITAL LETTER VE). Did you mean B (LATIN CAPITAL LETTER B)?
(RUF001)
348-348: String contains ambiguous с (CYRILLIC SMALL LETTER ES). Did you mean c (LATIN SMALL LETTER C)?
(RUF001)
348-348: String contains ambiguous е (CYRILLIC SMALL LETTER IE). Did you mean e (LATIN SMALL LETTER E)?
(RUF001)
348-348: String contains ambiguous г (CYRILLIC SMALL LETTER GHE). Did you mean r (LATIN SMALL LETTER R)?
(RUF001)
348-348: String contains ambiguous о (CYRILLIC SMALL LETTER O). Did you mean o (LATIN SMALL LETTER O)?
(RUF001)
354-354: f-string without any placeholders
Remove extraneous f prefix
(F541)
363-363: f-string without any placeholders
Remove extraneous f prefix
(F541)
372-372: f-string without any placeholders
Remove extraneous f prefix
(F541)
382-382: f-string without any placeholders
Remove extraneous f prefix
(F541)
382-382: String contains ambiguous с (CYRILLIC SMALL LETTER ES). Did you mean c (LATIN SMALL LETTER C)?
(RUF001)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (15)
- GitHub Check: QA
- GitHub Check: build (21, windows-latest)
- GitHub Check: build (23, macOS-latest)
- GitHub Check: build (17, macOS-latest)
- GitHub Check: build (17, windows-latest)
- GitHub Check: build (21, ubuntu-latest)
- GitHub Check: build (21, macOS-latest)
- GitHub Check: build (23, windows-latest)
- GitHub Check: build (17, ubuntu-latest)
- GitHub Check: build (23, ubuntu-latest)
- GitHub Check: build (17, windows-latest)
- GitHub Check: build (17, macOS-latest)
- GitHub Check: build (23, windows-latest)
- GitHub Check: build (21, macOS-latest)
- GitHub Check: build (21, windows-latest)
🔇 Additional comments (2)
src/jmh/java/com/github/_1c_syntax/bsl/mdclasses/benchmark/MDClassesBenchmark.java (2)
42-47: Конфигурация JMH выглядит корректно.Настройки бенчмарка (режим измерения, разогрев, количество итераций и форков) соответствуют лучшим практикам JMH для измерения производительности.
59-81: Методы бенчмарков реализованы корректно.Все четыре метода правильно используют
Blackholeдля предотвращения оптимизации мёртвого кода (DCE), а комбинации параметров (EDT/Designer × skipSupport) обеспечивают полное покрытие сценариев использования.
| for old_bench, new_bench in zip(old_data, new_data): | ||
| bench_name = old_bench['benchmark'].split('.')[-1] | ||
|
|
||
| # Производительность | ||
| old_time = old_bench['primaryMetric']['score'] | ||
| new_time = new_bench['primaryMetric']['score'] | ||
| change = ((old_time - new_time) / old_time) * 100 | ||
|
|
||
| performance_data.append({ | ||
| 'name': bench_name, | ||
| 'old': old_time, | ||
| 'new': new_time, | ||
| 'change': change | ||
| }) | ||
|
|
||
| # Память и GC из secondaryMetrics - УПРОЩЕННАЯ ФИЛЬТРАЦИЯ | ||
| old_secondary = old_bench.get('secondaryMetrics', {}) | ||
| new_secondary = new_bench.get('secondaryMetrics', {}) | ||
|
|
There was a problem hiding this comment.
Сопоставляйте результаты по имени бенчмарка, а не порядку.
zip(old_data, new_data) молча обрезает разновеликие списки и зависит от идентичного порядка, которого JMH не гарантирует (особенно при фильтрах/повторах). В итоге метрики могут сравниваться между разными тестами или вовсе теряться. Постройте словари по benchmark и объединяйте по ключу либо хотя бы контролируйте длину/состав перед сравнением.
🤖 Prompt for AI Agents
In benchmark-analyze-results.py around lines 27 to 45, the code uses
zip(old_data, new_data) which silently pairs entries by position and can
misalign or drop benchmarks when lists differ; instead build mappings keyed by
each item's 'benchmark' value (e.g., {bench['benchmark']: bench}) and iterate
over the sorted set of keys present in either map (or at least the
intersection), retrieving old/new entries by key, computing metrics only when
the corresponding entry exists, and emit warnings for missing benchmarks so
comparisons are correct and no results are accidentally compared out-of-order or
lost.
| if [[ -f "$RESULTS_DIR/sources.txt" ]]; then | ||
| source . "$RESULTS_DIR/sources.txt" | ||
| else | ||
| OLD_SOURCE="Ветка: $OLD_BRANCH" | ||
| NEW_SOURCE="Ветка: $NEW_BRANCH" | ||
| fi |
There was a problem hiding this comment.
Исправьте source и формат sources.txt.
Вызов source . "$RESULTS_DIR/sources.txt" пытается подключить каталог . и падает. Даже после исправления команда ничего не даст: файл содержит человекочитаемый текст, а не VAR=..., поэтому переменные не инициализируются. Уберите source и работайте с уже имеющимися $OLD_SOURCE/$NEW_SOURCE либо сохраните данные в пригодном для source формате.
🤖 Prompt for AI Agents
In benchmark-compare.sh around lines 183 to 188, the script incorrectly calls
`source . "$RESULTS_DIR/sources.txt"` which tries to source the current
directory and fails, and even if fixed the file contents are human-readable (not
VAR=...) so no variables are initialized; remove the `source` call and either
(A) stop sourcing and read/assign OLD_SOURCE and NEW_SOURCE from existing
variables ($OLD_SOURCE/$NEW_SOURCE) or (B) change the way sources.txt is written
so it contains shell-assignable lines like OLD_SOURCE='...' and NEW_SOURCE='...'
and then source that file; implement one of these two options and ensure
subsequent code uses the chosen variables accordingly.
| private final Path configPathEDT = Path.of("src/test/resources/ext/edt/ssl_3_1/configuration"); | ||
| private final Path configPathDesigner = Path.of("src/test/resources/ext/designer/ssl_3_1/src/cf"); |
There was a problem hiding this comment.
🧩 Analysis chain
Проверьте наличие путей к тестовым ресурсам.
Убедитесь, что указанные пути к конфигурациям существуют в репозитории и доступны при запуске бенчмарков.
Выполните следующий скрипт для проверки наличия путей:
🏁 Script executed:
#!/bin/bash
# Проверка существования путей к тестовым конфигурациям
if [ -d "src/test/resources/ext/edt/ssl_3_1/configuration" ]; then
echo "✓ EDT path exists"
else
echo "✗ EDT path NOT found: src/test/resources/ext/edt/ssl_3_1/configuration"
fi
if [ -d "src/test/resources/ext/designer/ssl_3_1/src/cf" ]; then
echo "✓ Designer path exists"
else
echo "✗ Designer path NOT found: src/test/resources/ext/designer/ssl_3_1/src/cf"
fiLength of output: 144
Исправьте пути к тестовым ресурсам: директории src/test/resources/ext/edt/ssl_3_1/configuration и src/test/resources/ext/designer/ssl_3_1/src/cf не найдены в репозитории.
🤖 Prompt for AI Agents
In
src/jmh/java/com/github/_1c_syntax/bsl/mdclasses/benchmark/MDClassesBenchmark.java
around lines 50-51, the hardcoded Paths for test resources point to non-existent
directories (src/test/resources/ext/edt/ssl_3_1/configuration and
src/test/resources/ext/designer/ssl_3_1/src/cf); update the code to reference
actual resource locations or load resources via the classloader (e.g., use
getResource/getResourceAsStream to locate files on the classpath and convert to
Path) or correct the string literals to the real relative paths in the
repository; if the resources are missing, add them to src/test/resources in the
expected structure and adjust the Paths accordingly.
|
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (5)
src/jmh/java/com/github/_1c_syntax/bsl/mdclasses/benchmark/MDClassesBenchmark.java (1)
50-51: Исправьте пути к тестовым ресурсам.Жёстко закодированные пути указывают на несуществующие директории
src/test/resources/ext/edt/ssl_3_1/configurationиsrc/test/resources/ext/designer/ssl_3_1/src/cf.Используйте один из подходов:
- Загрузите ресурсы через classloader (
getClass().getResource()) и преобразуйте вPath- Убедитесь, что директории существуют в структуре проекта
- Используйте относительные пути от корня репозитория, если ресурсы находятся в другом месте
- private final Path configPathEDT = Path.of("src/test/resources/ext/edt/ssl_3_1/configuration"); - private final Path configPathDesigner = Path.of("src/test/resources/ext/designer/ssl_3_1/src/cf"); + private final Path configPathEDT = Path.of( + getClass().getResource("/ext/edt/ssl_3_1/configuration").toURI() + ); + private final Path configPathDesigner = Path.of( + getClass().getResource("/ext/designer/ssl_3_1/src/cf").toURI() + );Либо добавьте ресурсы в
src/test/resourcesв ожидаемой структуре.benchmark-compare.sh (4)
121-121: Исправьте чтение несуществующего файла при использовании JAR.Когда
OLD_BRANCHявляется JAR-файлом, файл$RESULTS_DIR/old-version-commit.txtне создаётся (он создаётся только вbuild_from_branch), и командаcatзавершается ошибкой из-заset -e.Используйте условную проверку:
elif is_git_branch "$OLD_BRANCH"; then build_from_branch "$OLD_BRANCH" "old-version" OLD_SOURCE="Ветка: $OLD_BRANCH ($(cat $RESULTS_DIR/old-version-commit.txt))" else + # JAR файл + OLD_SOURCE="JAR файл: $(basename "$OLD_BRANCH")"Либо создавайте файл commit.txt в
use_existing_jarс информацией о JAR.
138-138: Исправьте чтение несуществующего файла при использовании JAR.Аналогично предыдущему комментарию: когда
NEW_BRANCHявляется JAR-файлом,catзавершится ошибкой.elif is_git_branch "$NEW_BRANCH"; then build_from_branch "$NEW_BRANCH" "new-version" NEW_SOURCE="Ветка: $NEW_BRANCH ($(cat $RESULTS_DIR/new-version-commit.txt))" else + # JAR файл + NEW_SOURCE="JAR файл: $(basename "$NEW_BRANCH")"
186-191: Исправьте командуsourceи формат файла.Команда
source . "$RESULTS_DIR/sources.txt"имеет две проблемы:
- Попытка выполнить
sourceдля каталога.(синтаксическая ошибка)- Файл
sources.txtсодержит человекочитаемый текст, а не переменные оболочки форматаVAR=...Удалите
source— переменные$OLD_SOURCEи$NEW_SOURCEуже инициализированы на строках 118/121 и 135/138:- # Загружаем информацию об источниках - if [[ -f "$RESULTS_DIR/sources.txt" ]]; then - source . "$RESULTS_DIR/sources.txt" - else - OLD_SOURCE="Ветка: $OLD_BRANCH" - NEW_SOURCE="Ветка: $NEW_BRANCH" - fi + # Переменные OLD_SOURCE и NEW_SOURCE уже установлены вышеЛибо измените строки 145-146, чтобы записывать переменные в формате для
source:echo "OLD_SOURCE='$OLD_SOURCE'" > "$RESULTS_DIR/sources.txt" echo "NEW_SOURCE='$NEW_SOURCE'" >> "$RESULTS_DIR/sources.txt"и исправьте
source:- source . "$RESULTS_DIR/sources.txt" + source "$RESULTS_DIR/sources.txt"
384-385: Исправьте чтение несуществующих файлов commit.txt.Функция
generate_reportпытается прочитатьold-version-commit.txtиnew-version-commit.txt, которые не существуют при использовании JAR-файлов.Используйте переменные
$OLD_SOURCEи$NEW_SOURCEвместо чтения файлов:echo "📈 ПОЛНЫЙ ОТЧЕТ СРАВНЕНИЯ MDClasses" > "$RESULTS_DIR/comparison-report.txt" echo "===================================" >> "$RESULTS_DIR/comparison-report.txt" - echo "Старая версия: $OLD_BRANCH ($(cat $RESULTS_DIR/old-version-commit.txt))" >> "$RESULTS_DIR/comparison-report.txt" - echo "Новая версия: $NEW_BRANCH ($(cat $RESULTS_DIR/new-version-commit.txt))" >> "$RESULTS_DIR/comparison-report.txt" + echo "Старая версия: $OLD_SOURCE" >> "$RESULTS_DIR/comparison-report.txt" + echo "Новая версия: $NEW_SOURCE" >> "$RESULTS_DIR/comparison-report.txt" echo "" >> "$RESULTS_DIR/comparison-report.txt"
🧹 Nitpick comments (2)
benchmark-compare.sh (2)
248-264: Рассмотрите обработку деления на ноль.Проверка
[[ "$old_score" != "0" ]]использует строковое сравнение. Для числовых данных с плавающей точкой это может дать неожиданные результаты (например,0.0не равно"0").Используйте числовое сравнение с
bc:- if command -v bc &> /dev/null && [[ "$old_score" != "0" ]]; then + if command -v bc &> /dev/null && (( $(echo "$old_score != 0" | bc -l) )); then change=$(echo "scale=2; (($old_score - $new_score) / $old_score) * 100" | bc 2>/dev/null)
328-344: Улучшите логику определения улучшения для метрик памяти.Условие
[[ "$metric" == memory* ]]проверяет префикс "memory", но метрикаgc.timeтакже означает улучшение при уменьшении, как и метрики памяти. Текущая логика обрабатывает GC-метрики в блокеelse, что корректно, но можно упростить.Рассмотрите более явную логику:
- # Для памяти и GC улучшение - уменьшение значений - if [[ "$metric" == memory* ]]; then + # Для всех метрик памяти и GC улучшение означает уменьшение + if [[ "$metric" == memory* ]] || [[ "$metric" == gc.time ]]; then if (( $(echo "$change > 0" | bc -l 2>/dev/null) )); then change_display="+${change}% ✅" elif (( $(echo "$change < 0" | bc -l 2>/dev/null) )); then change_display="${change}% ❌" else change_display="${change}% ➖" fi - else # GC метрики + elif [[ "$metric" == gc.collections ]]; then + # Для gc.collections: меньше коллекций - лучше if (( $(echo "$change < 0" | bc -l 2>/dev/null) )); thenЛибо добавьте комментарий, поясняющий текущую логику.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
benchmark-compare.sh(1 hunks)src/jmh/java/com/github/_1c_syntax/bsl/mdclasses/benchmark/MDClassesBenchmark.java(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (18)
- GitHub Check: build (21, macOS-latest)
- GitHub Check: build (23, windows-latest)
- GitHub Check: build (21, ubuntu-latest)
- GitHub Check: build (23, macOS-latest)
- GitHub Check: build (21, windows-latest)
- GitHub Check: build (17, macOS-latest)
- GitHub Check: build (17, ubuntu-latest)
- GitHub Check: build (23, ubuntu-latest)
- GitHub Check: build (17, windows-latest)
- GitHub Check: QA
- GitHub Check: build (23, windows-latest)
- GitHub Check: build (17, windows-latest)
- GitHub Check: build (23, macOS-latest)
- GitHub Check: build (23, ubuntu-latest)
- GitHub Check: build (17, ubuntu-latest)
- GitHub Check: build (17, macOS-latest)
- GitHub Check: build (21, windows-latest)
- GitHub Check: build (21, ubuntu-latest)
🔇 Additional comments (4)
src/jmh/java/com/github/_1c_syntax/bsl/mdclasses/benchmark/MDClassesBenchmark.java (2)
53-58: Отлично: оба пути теперь разогреваются.Метод
setup()теперь выполняет предварительную загрузку для обеих конфигураций (EDT и Designer), что устраняет асимметрию в начальных условиях бенчмарков.
60-82: Реализация бенчмарков корректна.Четыре метода бенчмарков правильно используют
Blackholeдля предотвращения оптимизации мёртвого кода, тестируют обе конфигурации (EDT/Designer) с обоими значениями флагаskipSupport.benchmark-compare.sh (2)
25-32: Корректная реализация проверки JAR-файла.Функция правильно проверяет расширение
.jarи существование файла.
35-47: Корректная проверка существования Git-ветки.Функция проверяет как локальные, так и удалённые ветки с помощью
git rev-parseиgit ls-remote.
1 participant
Описание
Добавлен тест производительности библиотеки на пример парсинга конфигурации БСП в двух вариантах исходников
Связанные задачи
Closes
Чеклист
Общие
gradlew precommit)Дополнительно