Defect #43446
openCommonMark rendering became significantly slower after #42737
0%
Description
After applying the changes from #42737, CommonMark rendering (issue descriptions, comments, wiki, etc.) has become much slower. Measuring before and after that change shows a slowdown of about 6x.
Since CommonMark rendering is used frequently across Redmine, this regression affects the overall responsiveness of the application.
Benchmark results¶
| Revision | Avg per render |
|---|---|
| r24093 (before #42737) | 23.90 ms |
| r24097 (after #42737) | 148.99 ms |
Benchmark script¶
bin/rails r commonmark_bench.rb
Files
Related issues
Updated by Katsuya HIDAKA 22 days ago
It turns out that the rendering becomes even slower when the CommonMark content is larger.
The following results are from running the same benchmark script used for the issue description, but with the CommonMark content tripled in size.
| Revision | Avg per render (10 times) |
|---|---|
| r24093 (before #42737) | 35.57 ms |
| r24094 (after #42737) | 1043.98 ms |
That's about 29.4x slower. From this result, the rendering performance appears to scale worse with larger content than it did before.
Benchmark script (with a slightly modified average calculation):
Updated by Takashi Kato 22 days ago
The performance issue was caused by SyntaxHighlightScrubber checking all nodes using a CSS selector and the `matches?` method.
The benchmark results after applying the patch in my environment are as follows:
| r24093: Average: | 34.94s ms |
| r24132: Average: | 1492.66s ms |
| patched: Average: | 48.18s ms |
While the performance is not completely back to its original speed, a significant performance degradation has been avoided.
Updated by Katsuya HIDAKA 19 days ago
Thank you for your patch.
I reviewed the patch posted in #note-3 and confirmed that it significantly improves performance. I also verified that CommonMark content, including alert syntax and syntax highlighting, renders correctly. Looks good to me.
Updated by Katsuya HIDAKA 19 days ago
I also confirmed that the patch in #note-3 passes all tests.
https://github.com/hidakatsuya/redmine/actions/runs/19221746428
In addition, I checked memory usage before and after applying the patch in #note-3 using memory_profiler , and found no significant difference.
Total allocated: 103200517 bytes (991840 objects) Total retained: 8750156 bytes (64822 objects)
Total allocated: 107467660 bytes (1000842 objects) Total retained: 8755362 bytes (64857 objects)
Updated by Go MAEDA 19 days ago
- Related to Patch #42737: Replacing html-pipeline with Loofah for HTML Filtering added