Do not include highlighted_diff_email css for each note for new review email
What does this MR do and why?
In #355027 (closed), we discovered that Premailer::Adapter::Nokogiri#to_inline_css
uses 25.93 seconds (51%) and it allocates a lot of memory: #355027 (comment 880321527)
After debugging Premailer::Adapter::Nokogiri#to_inline_css
, I found that the biggest issue is that we are including highlighted_diff_email.css
for each note/comment.
This means that in the case when a review contains 65 diff_discussion notes/comments, we will include mentioned CSS 65 times.
This will cause the Premailer Nokogiri adapter to search the whole HTML Dom object for each selector from that included css - (there are 81 of them). This action will then repeat 65 times, allocating 65x time more memory and time.
Premailer::Adapter::Nokogiri#to_inline_css
took total of 26.55 seconds to execute for 65 diff discussions.
Click to expand
selector | total time | count |
---|---|---|
.diff-line-num | 0.9784890040755272 | 65 |
.code > tr | 0.8081630114465952 | 65 |
.diff-line-num.new | 0.628456000238657 | 65 |
.line_content | 0.5463110134005547 | 65 |
.diff-line-num.old | 0.443707000464201 | 65 |
.line_content.new | 0.4226210117340088 | 65 |
.code | 0.42036799155175686 | 65 |
.n | .37347900308668613 | 65 |
.line_content.old | 0.3633930031210184 | 65 |
.o | 0.34669999964535236 | 65 |
.line_content.new > .line > span.idiff | 0.34396199882030487 | 65 |
.line_content.new > .line > span > span.idiff | 0.33864900656044483 | 65 |
.k | 0.33754899725317955 | 65 |
.line_content.old > .line > span > span.idiff | 0.3354520071297884 | 65 |
.line_content.old > .line > span.idiff | 0.3346880041062832 | 65 |
.line_content.match | 0.32732499577105045 | 65 |
.... | .... | .... |
This MR makes sure to include highlighted_diff_email
css only once.
Results with fix
Screenshots or screen recordings
These are strongly recommended to assist reviewers and reduce the time to merge your change.
How to set up and validate locally highlighted_diff_email
Numbered steps to set up and validate the change are strongly suggested.
MR acceptance checklist
This checklist encourages us to confirm any changes have been analyzed to reduce risks in quality, performance, reliability, security, and maintainability.
-
I have evaluated the MR acceptance checklist for this MR.
Related to #355027 (closed)