Skip to content

Commit

Permalink
More optimizations (#55)
Browse files Browse the repository at this point in the history
* Let Match be a struct.

It's immutable and one is created per route match. Benchmark is updated
as follows:

Before:
router 541.14k (  1.85µs) (± 3.28%)  2.02kB/op  fastest

After:
router 594.45k (  1.68µs) (± 1.06%)  1.83kB/op  fastest

* Use StaticArray for segments to avoid memory allocation

The benchmark times are updated as follows:

Before:
router 590.63k (  1.69µs) (± 1.06%)  1.83kB/op  fastest

After:
router 720.56k (  1.39µs) (± 1.16%)  1.33kB/op  fastest

* No need to use Char::Reader in PathReader

Given that we are only looking for the ascii characters '/' and '%', and
UTF-8 guarantees that these will always come as individual bytes in the
beginning of a character, we can simply traverse the string by bytes.
This avoids decoding UTF-8 and gives a small performance boost.

Benchmark times are updated as follows:

Before:
router 720.56k (  1.39µs) (± 1.16%)  1.33kB/op  fastest

After:
router 767.78k (  1.30µs) (± 3.48%)  1.33kB/op  fastest

* Avoid variable shadowing

* Avoid using `to_unsafe`

* Don't use `uninitialized`

This should be safer, and it seems performance isn't affected.

* Make ameba happy
  • Loading branch information
asterite authored Sep 16, 2021
1 parent 11bdf52 commit 0c073c1
Show file tree
Hide file tree
Showing 5 changed files with 71 additions and 13 deletions.
9 changes: 9 additions & 0 deletions spec/integration_spec.cr
Original file line number Diff line number Diff line change
Expand Up @@ -194,6 +194,15 @@ describe LuckyRouter do
})
end

it "matches a route with more than 16 segments" do
router = LuckyRouter::Matcher(Symbol).new
router.add("get", "/a/b/c/d/e/f/g/h/i/j/k/l/m/n/o/p/q/r/s/t/u/v/w/x/y/:z", :match)

match = router.match!("get", "/a/b/c/d/e/f/g/h/i/j/k/l/m/n/o/p/q/r/s/t/u/v/w/x/y/z")
match.payload.should eq(:match)
match.params["z"].should eq("z")
end

describe "route with trailing slash" do
context "is defined with a trailing slash" do
it "should treat it as a index route when called without a trailing slash" do
Expand Down
20 changes: 17 additions & 3 deletions src/lucky_router/fragment.cr
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,11 @@ class LuckyRouter::Fragment(T)
find_match(parts, method) || NoMatch.new
end

# :ditto:
def find(parts : Slice(String), method : String) : Match(T) | NoMatch
find_match(parts, method) || NoMatch.new
end

def process_parts(parts : Array(PathPart), method : String, payload : T)
leaf_fragment = parts.reduce(self) { |fragment, part| fragment.add_part(part) }
leaf_fragment.method_to_payload[method] = payload
Expand Down Expand Up @@ -84,16 +89,19 @@ class LuckyRouter::Fragment(T)
find_match(path_parts, 0, method)
end

def find_match(path_parts : Slice(String), method : String) : Match(T)?
find_match(path_parts, 0, method)
end

def match_for_method(method)
payload = method_to_payload[method]?
payload ? Match(T).new(payload, Hash(String, String).new) : nil
end

protected def find_match(path_parts : Array(String), index : Int32, method : String) : Match(T)?
protected def find_match(path_parts, index, method : String) : Match(T)?
return match_for_method(method) if index >= path_parts.size

path_part = path_parts[index]

index += 1

find_match_with_static_parts(path_part, path_parts, index, method) ||
Expand Down Expand Up @@ -122,7 +130,13 @@ class LuckyRouter::Fragment(T)
return unless glob

if match = glob.match_for_method(method)
match.params[glob.path_part.name] = path_parts[index..].unshift(path_part).join("/")
match.params[glob.path_part.name] = String.build do |io|
io << path_part
index.upto(path_parts.size - 1) do |sub_index|
io << '/'
io << path_parts[sub_index]
end
end
match
end
end
Expand Down
2 changes: 1 addition & 1 deletion src/lucky_router/match.cr
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
class LuckyRouter::Match(T)
struct LuckyRouter::Match(T)
getter payload : T
getter params : Hash(String, String)

Expand Down
34 changes: 32 additions & 2 deletions src/lucky_router/matcher.cr
Original file line number Diff line number Diff line change
Expand Up @@ -65,8 +65,38 @@ class LuckyRouter::Matcher(T)
end

def match(method : String, path_to_match : String) : Match(T)?
parts = LuckerRouter::PathReader.new(path_to_match).to_a
match = root.find(parts, method)
# To avoid allocating an array for the segment parts, we use a static
# array with up to 16 segments.
parts_static_array = StaticArray(String, 16).new("")

# In the general case we still have to support more than 16 segments.
# We'll fallback to using an Array for that case.
parts_array = nil

index = 0
LuckerRouter::PathReader.new(path_to_match).each do |part|
if index == parts_static_array.size
# We don't have any more space in the static array:
# more contents to the array.
parts_array = Array(String).new(32)
parts_array.concat(parts_static_array)
parts_array << part
elsif parts_array
# We are using the fallback array, so push parts there.
parts_array << part
else
# We are still using the static array
parts_static_array[index] = part
end
index += 1
end

match =
if parts_array
root.find(parts_array, method)
else
root.find(parts_static_array.to_slice[0...index], method)
end

if match.is_a?(Match)
match
Expand Down
19 changes: 12 additions & 7 deletions src/lucky_router/path_reader.cr
Original file line number Diff line number Diff line change
Expand Up @@ -35,20 +35,25 @@ struct LuckerRouter::PathReader
end

private def each_segment
decode = false
index = 0
offset = 0
decode = false
slice = @path.to_slice

reader = Char::Reader.new(@path)
reader.each do |char|
case char
while index < slice.size
byte = slice[index]
case byte
when '/'
length = reader.pos - offset
length = index - offset
yield offset, length, decode
decode = false
offset = reader.pos + 1
index += 1
offset = index
when '%'
decode = true
reader.pos += 2
index += 3
else
index += 1
end
end

Expand Down

0 comments on commit 0c073c1

Please sign in to comment.