Skip to content

Commit

Permalink
Improve performance of include? by 5-10x
Browse files Browse the repository at this point in the history
Rails uses IPAddr#include? to evaluate what it should use as the
client's remote ip by filtering potential ips against a trusted list
of internal ips. In a _very_ minimal app, #include? was showing up in
a profile as ~1% of request time.

The issue is that #include? was converting itself and the other value
passed in to ranges of IPAddr. This mean as a worst case (where other is
a non-IPAddr, like a String) then there would be 5 IPAddr instances
created (other -> IPAddr, and two each for the conversions to ranges).
However, wrapping the begin and end values as IPAddr is not needed
because they are necessarily fixed addresses already.

This patch extracts the logic for getting the begin_addr and end_addr
from the #to_range method so that they can be used in #include? without
having to instantiate so many IPAddr.

Benchmark:

```ruby
net1 = IPAddr.new("192.168.2.0/24")
net2 = IPAddr.new("192.168.2.100")
net3 = IPAddr.new("192.168.3.0")
net4 = IPAddr.new("192.168.2.0/16")

Benchmark.ips do |x|
  x.report("/24 includes address") { net1.include? net2 }
  x.report("/24 not includes address") { net1.include? net3 }
  x.report("/16 includes /24") { net4.include? net1 }
  x.report("/24 not includes /16") { net1.include? net4 }
  x.compare!
end
```

Before:

```
Comparison:
    /24 not includes /16:   175041.3 i/s
/24 not includes address:   164933.2 i/s - 1.06x  (± 0.00) slower
        /16 includes /24:   163881.9 i/s - 1.07x  (± 0.00) slower
    /24 includes address:   163558.4 i/s - 1.07x  (± 0.00) slower
```

After:

```
Comparison:
    /24 not includes /16:  2588364.9 i/s
/24 not includes address:  1474650.7 i/s - 1.76x  (± 0.00) slower
        /16 includes /24:  1461351.0 i/s - 1.77x  (± 0.00) slower
    /24 includes address:  1425463.5 i/s - 1.82x  (± 0.00) slower
```
  • Loading branch information
skipkayhil authored and matzbot committed Sep 23, 2023
1 parent 2ceb536 commit e581b78
Showing 1 changed file with 16 additions and 14 deletions.
30 changes: 16 additions & 14 deletions lib/ipaddr.rb
Expand Up @@ -176,9 +176,7 @@ def mask(prefixlen)
def include?(other)
other = coerce_other(other)
return false unless other.family == family
range = to_range
other = other.to_range
range.begin <= other.begin && range.end >= other.end
begin_addr <= other.begin_addr && end_addr >= other.end_addr
end
alias === include?

Expand Down Expand Up @@ -406,17 +404,6 @@ def hash

# Creates a Range object for the network address.
def to_range
begin_addr = (@addr & @mask_addr)

case @family
when Socket::AF_INET
end_addr = (@addr | (IN4MASK ^ @mask_addr))
when Socket::AF_INET6
end_addr = (@addr | (IN6MASK ^ @mask_addr))
else
raise AddressFamilyError, "unsupported address family"
end

self.class.new(begin_addr, @family)..self.class.new(end_addr, @family)
end

Expand Down Expand Up @@ -497,6 +484,21 @@ def zone_id=(zid)

protected

def begin_addr
@addr & @mask_addr
end

def end_addr
case @family
when Socket::AF_INET
@addr | (IN4MASK ^ @mask_addr)
when Socket::AF_INET6
@addr | (IN6MASK ^ @mask_addr)
else
raise AddressFamilyError, "unsupported address family"
end
end

# Set +@addr+, the internal stored ip address, to given +addr+. The
# parameter +addr+ is validated using the first +family+ member,
# which is +Socket::AF_INET+ or +Socket::AF_INET6+.
Expand Down

0 comments on commit e581b78

Please sign in to comment.