Commit fc9f0998 authored by Lukas '+ alert('Eipi') + ' Eipert's avatar Lukas '+ alert('Eipi') + ' Eipert Committed by Fatih Acet

Prevent empty classes in ansi2html conversion

Currently we write out empty CSS classes (`class=""`) every time we
create a new tag. This adds 9 unnecessary bytes per span element. In a
recent trace, I have counted 11950 span elements. So we transported 105
unnecessary kilobytes!
parent 0fec9a4f
...@@ -127,6 +127,7 @@ ...@@ -127,6 +127,7 @@
.section-header ~ .section.line { .section-header ~ .section.line {
margin-left: $gl-padding; margin-left: $gl-padding;
display: block;
} }
} }
......
---
title: Improve job log rendering performance
merge_request: 31262
author:
type: performance
...@@ -194,16 +194,10 @@ module Gitlab ...@@ -194,16 +194,10 @@ module Gitlab
end end
def handle_new_line def handle_new_line
css_classes = []
if @sections.any?
css_classes = %w[section line] + sections.map { |section| "s_#{section}" }
end
write_in_tag %{<br/>} write_in_tag %{<br/>}
write_raw %{<span class="#{css_classes.join(' ')}"></span>} if css_classes.any?
close_open_tags if @sections.any? && @lineno_in_section == 0
@lineno_in_section += 1 @lineno_in_section += 1
open_new_tag
end end
def handle_section(scanner) def handle_section(scanner)
...@@ -310,11 +304,24 @@ module Gitlab ...@@ -310,11 +304,24 @@ module Gitlab
if @sections.any? if @sections.any?
css_classes << "section" css_classes << "section"
css_classes << "js-section-header section-header" if @lineno_in_section == 0
css_classes << if @lineno_in_section == 0
"js-section-header section-header"
else
"line"
end
css_classes += sections.map { |section| "js-s-#{section}" } css_classes += sections.map { |section| "js-s-#{section}" }
end end
@out << %{<span class="#{css_classes.join(' ')}">} close_open_tags
@out << if css_classes.any?
%{<span class="#{css_classes.join(' ')}">}
else
%{<span>}
end
@n_open_tags += 1 @n_open_tags += 1
end end
......
...@@ -546,7 +546,7 @@ describe Projects::JobsController, :clean_gitlab_redis_shared_state do ...@@ -546,7 +546,7 @@ describe Projects::JobsController, :clean_gitlab_redis_shared_state do
expect(response).to have_gitlab_http_status(:ok) expect(response).to have_gitlab_http_status(:ok)
expect(json_response['id']).to eq job.id expect(json_response['id']).to eq job.id
expect(json_response['status']).to eq job.status expect(json_response['status']).to eq job.status
expect(json_response['html']).to eq('<span class="">BUILD TRACE</span>') expect(json_response['html']).to eq('<span>BUILD TRACE</span>')
end end
end end
......
...@@ -6,11 +6,11 @@ describe Gitlab::Ci::Ansi2html do ...@@ -6,11 +6,11 @@ describe Gitlab::Ci::Ansi2html do
subject { described_class } subject { described_class }
it "prints non-ansi as-is" do it "prints non-ansi as-is" do
expect(convert_html("Hello")).to eq('<span class="">Hello</span>') expect(convert_html("Hello")).to eq('<span>Hello</span>')
end end
it "strips non-color-changing control sequences" do it "strips non-color-changing control sequences" do
expect(convert_html("Hello \e[2Kworld")).to eq('<span class="">Hello world</span>') expect(convert_html("Hello \e[2Kworld")).to eq('<span>Hello world</span>')
end end
it "prints simply red" do it "prints simply red" do
...@@ -34,7 +34,7 @@ describe Gitlab::Ci::Ansi2html do ...@@ -34,7 +34,7 @@ describe Gitlab::Ci::Ansi2html do
end end
it "resets colors after red on blue" do it "resets colors after red on blue" do
expect(convert_html("\e[31;44mHello\e[0m world")).to eq('<span class="term-fg-red term-bg-blue">Hello</span><span class=""> world</span>') expect(convert_html("\e[31;44mHello\e[0m world")).to eq('<span class="term-fg-red term-bg-blue">Hello</span><span> world</span>')
end end
it "performs color change from red/blue to yellow/blue" do it "performs color change from red/blue to yellow/blue" do
...@@ -46,11 +46,11 @@ describe Gitlab::Ci::Ansi2html do ...@@ -46,11 +46,11 @@ describe Gitlab::Ci::Ansi2html do
end end
it "performs color change from red/blue to reset to yellow/green" do it "performs color change from red/blue to reset to yellow/green" do
expect(convert_html("\e[31;44mHello\e[0m \e[33;42mworld")).to eq('<span class="term-fg-red term-bg-blue">Hello</span><span class=""> </span><span class="term-fg-yellow term-bg-green">world</span>') expect(convert_html("\e[31;44mHello\e[0m \e[33;42mworld")).to eq('<span class="term-fg-red term-bg-blue">Hello</span><span> </span><span class="term-fg-yellow term-bg-green">world</span>')
end end
it "ignores unsupported codes" do it "ignores unsupported codes" do
expect(convert_html("\e[51mHello\e[0m")).to eq('<span class="">Hello</span>') expect(convert_html("\e[51mHello\e[0m")).to eq('<span>Hello</span>')
end end
it "prints light red" do it "prints light red" do
...@@ -74,8 +74,8 @@ describe Gitlab::Ci::Ansi2html do ...@@ -74,8 +74,8 @@ describe Gitlab::Ci::Ansi2html do
end end
it "resets bold text" do it "resets bold text" do
expect(convert_html("\e[1mHello\e[21m world")).to eq('<span class="term-bold">Hello</span><span class=""> world</span>') expect(convert_html("\e[1mHello\e[21m world")).to eq('<span class="term-bold">Hello</span><span> world</span>')
expect(convert_html("\e[1mHello\e[22m world")).to eq('<span class="term-bold">Hello</span><span class=""> world</span>') expect(convert_html("\e[1mHello\e[22m world")).to eq('<span class="term-bold">Hello</span><span> world</span>')
end end
it "prints italic text" do it "prints italic text" do
...@@ -83,7 +83,7 @@ describe Gitlab::Ci::Ansi2html do ...@@ -83,7 +83,7 @@ describe Gitlab::Ci::Ansi2html do
end end
it "resets italic text" do it "resets italic text" do
expect(convert_html("\e[3mHello\e[23m world")).to eq('<span class="term-italic">Hello</span><span class=""> world</span>') expect(convert_html("\e[3mHello\e[23m world")).to eq('<span class="term-italic">Hello</span><span> world</span>')
end end
it "prints underlined text" do it "prints underlined text" do
...@@ -91,7 +91,7 @@ describe Gitlab::Ci::Ansi2html do ...@@ -91,7 +91,7 @@ describe Gitlab::Ci::Ansi2html do
end end
it "resets underlined text" do it "resets underlined text" do
expect(convert_html("\e[4mHello\e[24m world")).to eq('<span class="term-underline">Hello</span><span class=""> world</span>') expect(convert_html("\e[4mHello\e[24m world")).to eq('<span class="term-underline">Hello</span><span> world</span>')
end end
it "prints concealed text" do it "prints concealed text" do
...@@ -99,7 +99,7 @@ describe Gitlab::Ci::Ansi2html do ...@@ -99,7 +99,7 @@ describe Gitlab::Ci::Ansi2html do
end end
it "resets concealed text" do it "resets concealed text" do
expect(convert_html("\e[8mHello\e[28m world")).to eq('<span class="term-conceal">Hello</span><span class=""> world</span>') expect(convert_html("\e[8mHello\e[28m world")).to eq('<span class="term-conceal">Hello</span><span> world</span>')
end end
it "prints crossed-out text" do it "prints crossed-out text" do
...@@ -107,7 +107,7 @@ describe Gitlab::Ci::Ansi2html do ...@@ -107,7 +107,7 @@ describe Gitlab::Ci::Ansi2html do
end end
it "resets crossed-out text" do it "resets crossed-out text" do
expect(convert_html("\e[9mHello\e[29m world")).to eq('<span class="term-cross">Hello</span><span class=""> world</span>') expect(convert_html("\e[9mHello\e[29m world")).to eq('<span class="term-cross">Hello</span><span> world</span>')
end end
it "can print 256 xterm fg colors" do it "can print 256 xterm fg colors" do
...@@ -139,15 +139,15 @@ describe Gitlab::Ci::Ansi2html do ...@@ -139,15 +139,15 @@ describe Gitlab::Ci::Ansi2html do
end end
it "prints &lt;" do it "prints &lt;" do
expect(convert_html("<")).to eq('<span class="">&lt;</span>') expect(convert_html("<")).to eq('<span>&lt;</span>')
end end
it "replaces newlines with line break tags" do it "replaces newlines with line break tags" do
expect(convert_html("\n")).to eq('<span class=""><br/><span class=""></span></span>') expect(convert_html("\n")).to eq('<span><br/></span>')
end end
it "groups carriage returns with newlines" do it "groups carriage returns with newlines" do
expect(convert_html("\r\n")).to eq('<span class=""><br/><span class=""></span></span>') expect(convert_html("\r\n")).to eq('<span><br/></span>')
end end
describe "incremental update" do describe "incremental update" do
...@@ -184,7 +184,7 @@ describe Gitlab::Ci::Ansi2html do ...@@ -184,7 +184,7 @@ describe Gitlab::Ci::Ansi2html do
context "with partial sequence" do context "with partial sequence" do
let(:pre_text) { "Hello\e" } let(:pre_text) { "Hello\e" }
let(:pre_html) { "<span class=\"\">Hello</span>" } let(:pre_html) { "<span>Hello</span>" }
let(:text) { "[1m World" } let(:text) { "[1m World" }
let(:html) { "<span class=\"term-bold\"> World</span>" } let(:html) { "<span class=\"term-bold\"> World</span>" }
...@@ -193,9 +193,9 @@ describe Gitlab::Ci::Ansi2html do ...@@ -193,9 +193,9 @@ describe Gitlab::Ci::Ansi2html do
context 'with new line' do context 'with new line' do
let(:pre_text) { "Hello\r" } let(:pre_text) { "Hello\r" }
let(:pre_html) { "<span class=\"\">Hello\r</span>" } let(:pre_html) { "<span>Hello\r</span>" }
let(:text) { "\nWorld" } let(:text) { "\nWorld" }
let(:html) { "<span class=\"\"><br/><span class=\"\">World</span></span>" } let(:html) { "<span><br/>World</span>" }
it_behaves_like 'stateable converter' it_behaves_like 'stateable converter'
end end
...@@ -222,7 +222,7 @@ describe Gitlab::Ci::Ansi2html do ...@@ -222,7 +222,7 @@ describe Gitlab::Ci::Ansi2html do
text = "#{section_start}Some text#{section_end}" text = "#{section_start}Some text#{section_end}"
class_name_start = section_start.gsub("\033[0K", '').gsub('<', '&lt;') class_name_start = section_start.gsub("\033[0K", '').gsub('<', '&lt;')
class_name_end = section_end.gsub("\033[0K", '').gsub('<', '&lt;') class_name_end = section_end.gsub("\033[0K", '').gsub('<', '&lt;')
html = %{<span class="">#{class_name_start}Some text#{class_name_end}</span>} html = %{<span>#{class_name_start}Some text#{class_name_end}</span>}
expect(convert_html(text)).to eq(html) expect(convert_html(text)).to eq(html)
end end
...@@ -232,12 +232,11 @@ describe Gitlab::Ci::Ansi2html do ...@@ -232,12 +232,11 @@ describe Gitlab::Ci::Ansi2html do
let(:text) { "#{section_start}Some text#{section_end}" } let(:text) { "#{section_start}Some text#{section_end}" }
it 'prints light red' do it 'prints light red' do
text = "#{section_start}\e[91mHello\e[0m\n#{section_end}" text = "#{section_start}\e[91mHello\e[0m\nLine 1\nLine 2\nLine 3\n#{section_end}"
header = %{<span class="term-fg-l-red section js-section-header section-header js-s-#{class_name(section_name)}">Hello</span>} header = %{<span class="term-fg-l-red section js-section-header section-header js-s-#{class_name(section_name)}">Hello</span>}
line_break = %{<span class="section js-section-header section-header js-s-#{class_name(section_name)}"><br/></span>} line_break = %{<span class="section js-section-header section-header js-s-#{class_name(section_name)}"><br/></span>}
line = %{<span class="section line s_#{class_name(section_name)}"></span>} output_line = %{<span class="section line js-s-#{class_name(section_name)}">Line 1<br/>Line 2<br/>Line 3<br/></span>}
empty_line = %{<span class="section js-s-#{class_name(section_name)}"></span>} html = "#{section_start_html}#{header}#{line_break}#{output_line}#{section_end_html}"
html = "#{section_start_html}#{header}#{line_break}#{line}#{empty_line}#{section_end_html}"
expect(convert_html(text)).to eq(html) expect(convert_html(text)).to eq(html)
end end
......
...@@ -65,9 +65,9 @@ describe Gitlab::Ci::Trace::Stream, :clean_gitlab_redis_cache do ...@@ -65,9 +65,9 @@ describe Gitlab::Ci::Trace::Stream, :clean_gitlab_redis_cache do
result = stream.html result = stream.html
expect(result).to eq( expect(result).to eq(
"<span class=\"\">ヾ(´༎ຶД༎ຶ`)ノ<br/><span class=\"\"></span></span>"\ "<span>ヾ(´༎ຶД༎ຶ`)ノ<br/></span>"\
"<span class=\"term-fg-green\">許功蓋</span><span class=\"\"><br/>"\ "<span class=\"term-fg-green\">許功蓋</span>"\
"<span class=\"\"></span></span>") "<span><br/></span>")
expect(result.encoding).to eq(Encoding.default_external) expect(result.encoding).to eq(Encoding.default_external)
end end
end end
...@@ -253,7 +253,7 @@ describe Gitlab::Ci::Trace::Stream, :clean_gitlab_redis_cache do ...@@ -253,7 +253,7 @@ describe Gitlab::Ci::Trace::Stream, :clean_gitlab_redis_cache do
it 'returns html content with state' do it 'returns html content with state' do
result = stream.html_with_state result = stream.html_with_state
expect(result.html).to eq("<span class=\"\">1234</span>") expect(result.html).to eq("<span>1234</span>")
end end
context 'follow-up state' do context 'follow-up state' do
...@@ -269,7 +269,7 @@ describe Gitlab::Ci::Trace::Stream, :clean_gitlab_redis_cache do ...@@ -269,7 +269,7 @@ describe Gitlab::Ci::Trace::Stream, :clean_gitlab_redis_cache do
result = stream.html_with_state(last_result.state) result = stream.html_with_state(last_result.state)
expect(result.append).to be_truthy expect(result.append).to be_truthy
expect(result.html).to eq("<span class=\"\">5678</span>") expect(result.html).to eq("<span>5678</span>")
end end
end end
end end
...@@ -305,13 +305,11 @@ describe Gitlab::Ci::Trace::Stream, :clean_gitlab_redis_cache do ...@@ -305,13 +305,11 @@ describe Gitlab::Ci::Trace::Stream, :clean_gitlab_redis_cache do
describe '#html' do describe '#html' do
shared_examples_for 'htmls' do shared_examples_for 'htmls' do
it "returns html" do it "returns html" do
expect(stream.html).to eq( expect(stream.html).to eq("<span>12<br/>34<br/>56</span>")
"<span class=\"\">12<br/><span class=\"\">34<br/>"\
"<span class=\"\">56</span></span></span>")
end end
it "returns html for last line only" do it "returns html for last line only" do
expect(stream.html(last_lines: 1)).to eq("<span class=\"\">56</span>") expect(stream.html(last_lines: 1)).to eq("<span>56</span>")
end end
end end
......
...@@ -7,11 +7,11 @@ shared_examples_for 'common trace features' do ...@@ -7,11 +7,11 @@ shared_examples_for 'common trace features' do
end end
it "returns formatted html" do it "returns formatted html" do
expect(trace.html).to eq("<span class=\"\">12<br/><span class=\"\">34</span></span>") expect(trace.html).to eq("<span>12<br/>34</span>")
end end
it "returns last line of formatted html" do it "returns last line of formatted html" do
expect(trace.html(last_lines: 1)).to eq("<span class=\"\">34</span>") expect(trace.html(last_lines: 1)).to eq("<span>34</span>")
end end
end end
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment