Skip to content

Commit 1fd0ae5

Browse files
committed
Fix perf issue with combination
After dead_end has determined that the problem has been found, it then goes back through all it's code blocks to determine the smallest set of blocks which hold all the errors (since there can be multiple errors and multiple blocks could hold the same error). It does this by building a "combination" of all possible block results and then checking each. To see, look at this spec: ```ruby expect( CodeFrontier.combination([:a, :b, :c, :d]) ).to eq( [ [:a],[:b],[:c],[:d], [:a, :b], [:a, :c], [:a, :d], [:b, :c], [:b, :d], [:c, :d], [:a, :b, :c], [:a, :b, :d], [:a, :c, :d], [:b, :c, :d], [:a, :b, :c, :d] ] ) ``` Imagine that :a, :b, :c, and :d are each code blocks. If we have 4 blocks in our frontier it generates 15 possibilities to evaluate. - 4 => 15 - 5 => 31 - 6 => 63 - 7 => 127 In this failing scenario there are a large number of blocks 23 blocks to be exact. - 23 => 8,388,607 It takes 4.7 seconds just to generate all these combinations with numeric values, which is not sustainable. This doesn't even include the time to perform the parse check 8 million times, just to generate 8 million arrays with all possible combinations. YIKEs. This wasn't previously as issue because as code blocks "expand" they consolidate when a larger block contains smaller blocks they get grouped together. The reason this example is so "bad", is that there's a lot of blocks all at the same indentation level, so they've not had a chance to consolidate yet. The fix is to evaluate fewer combinations. We know that removing valid code from an invalid document cannot create a valid document, so we can remove all blocks that are already valid. When we've filtered to only invalid blocks, there's only 1 in the frontier which comes back almost instantaneously. This patch reduces the time to find the syntax error from about 9 seconds to 0.03 seconds. Not bad
1 parent 942c20e commit 1fd0ae5

File tree

5 files changed

+159
-2
lines changed

5 files changed

+159
-2
lines changed

CHANGELOG.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
## HEAD (unreleased)
22

3+
- Fix performance issue when evaluating multiple block combinations ()
4+
35
## 1.0.0
46

57
- Gem name changed from `syntax_search` to `dead_end` (https://github.com/zombocom/syntax_search/pull/30)

lib/dead_end/code_frontier.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -143,7 +143,7 @@ def self.combination(array)
143143
# Given that we know our syntax error exists somewhere in our frontier, we want to find
144144
# the smallest possible set of blocks that contain all the syntax errors
145145
def detect_invalid_blocks
146-
self.class.combination(@frontier).detect do |block_array|
146+
self.class.combination(@frontier.select(&:invalid?)).detect do |block_array|
147147
holds_all_syntax_errors?(block_array)
148148
end || []
149149
end

lib/dead_end/internals.rb

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,8 +53,9 @@ def self.call(source: , filename: , terminal: false, record_dir: nil, timeout: T
5353
invalid_obj: invalid_type(source),
5454
io: io
5555
).call
56-
rescue Timeout::Error
56+
rescue Timeout::Error => e
5757
io.puts "Search timed out DEAD_END_TIMEOUT=#{timeout}, run with DEBUG=1 for more info"
58+
io.puts e.backtrace.first(3).join($/)
5859
end
5960

6061
# Used for counting spaces

spec/fixtures/routes.txt.rb

Lines changed: 121 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,121 @@
1+
Rails.application.routes.draw do
2+
constraints -> { Rails.application.config.non_production } do
3+
namespace :foo do
4+
resource :bar
5+
end
6+
end
7+
constraints -> { Rails.application.config.non_production } do
8+
namespace :bar do
9+
resource :baz
10+
end
11+
end
12+
constraints -> { Rails.application.config.non_production } do
13+
namespace :bar do
14+
resource :baz
15+
end
16+
end
17+
constraints -> { Rails.application.config.non_production } do
18+
namespace :bar do
19+
resource :baz
20+
end
21+
end
22+
constraints -> { Rails.application.config.non_production } do
23+
namespace :bar do
24+
resource :baz
25+
end
26+
end
27+
constraints -> { Rails.application.config.non_production } do
28+
namespace :bar do
29+
resource :baz
30+
end
31+
end
32+
constraints -> { Rails.application.config.non_production } do
33+
namespace :bar do
34+
resource :baz
35+
end
36+
end
37+
constraints -> { Rails.application.config.non_production } do
38+
namespace :bar do
39+
resource :baz
40+
end
41+
end
42+
constraints -> { Rails.application.config.non_production } do
43+
namespace :bar do
44+
resource :baz
45+
end
46+
end
47+
constraints -> { Rails.application.config.non_production } do
48+
namespace :bar do
49+
resource :baz
50+
end
51+
end
52+
constraints -> { Rails.application.config.non_production } do
53+
namespace :bar do
54+
resource :baz
55+
end
56+
end
57+
constraints -> { Rails.application.config.non_production } do
58+
namespace :bar do
59+
resource :baz
60+
end
61+
end
62+
constraints -> { Rails.application.config.non_production } do
63+
namespace :bar do
64+
resource :baz
65+
end
66+
end
67+
constraints -> { Rails.application.config.non_production } do
68+
namespace :bar do
69+
resource :baz
70+
end
71+
end
72+
constraints -> { Rails.application.config.non_production } do
73+
namespace :bar do
74+
resource :baz
75+
end
76+
end
77+
constraints -> { Rails.application.config.non_production } do
78+
namespace :bar do
79+
resource :baz
80+
end
81+
end
82+
constraints -> { Rails.application.config.non_production } do
83+
namespace :bar do
84+
resource :baz
85+
end
86+
end
87+
constraints -> { Rails.application.config.non_production } do
88+
namespace :bar do
89+
resource :baz
90+
end
91+
end
92+
constraints -> { Rails.application.config.non_production } do
93+
namespace :bar do
94+
resource :baz
95+
end
96+
end
97+
constraints -> { Rails.application.config.non_production } do
98+
namespace :bar do
99+
resource :baz
100+
end
101+
end
102+
constraints -> { Rails.application.config.non_production } do
103+
namespace :bar do
104+
resource :baz
105+
end
106+
end
107+
constraints -> { Rails.application.config.non_production } do
108+
namespace :bar do
109+
resource :baz
110+
end
111+
end
112+
113+
namespace :admin do
114+
resource :session
115+
116+
match "/out_of_office(*path)", via: :all, to: redirect { |_params, req|
117+
uri = URI(req.path.gsub("out_of_office", "in_office"))
118+
uri.query = req.query_string.presence
119+
uri.to_s
120+
}
121+
end

spec/perf/perf_spec.rb

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
# frozen_string_literal: true
2+
3+
require_relative "../spec_helper.rb"
4+
require "benchmark"
5+
6+
module DeadEnd
7+
RSpec.describe "perf" do
8+
it "doesnt timeout" do
9+
source = fixtures_dir.join("routes.txt.rb").read
10+
11+
io = StringIO.new
12+
bench = Benchmark.measure do
13+
DeadEnd.call(
14+
io: io,
15+
source: source,
16+
filename: "none",
17+
)
18+
end
19+
20+
expect(io.string).to include(<<~'EOM'.indent(4))
21+
1 Rails.application.routes.draw do
22+
107 constraints -> { Rails.application.config.non_production } do
23+
111 end
24+
❯ 113 namespace :admin do
25+
❯ 116 match "/out_of_office(*path)", via: :all, to: redirect { |_params, req|
26+
❯ 120 }
27+
121 end
28+
EOM
29+
30+
expect(bench.real).to be < 1 # second
31+
end
32+
end
33+
end

0 commit comments

Comments
 (0)