Skip to content

Small optimizations #308

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 5 commits into from
Closed

Small optimizations #308

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
2db0708
Switch accessor creation from define_method to class_eval
film42 Feb 28, 2016
c6e6262
Use a slighly more performant version of VarintPure decode
film42 Feb 28, 2016
6f007d7
Use != and == over nonzero? and zero? in hot code paths for better speed
film42 Feb 28, 2016
48e06f8
Alpha-order gemspec dependencies
film42 Feb 28, 2016
6efe052
Fix define_accessors to work with full qualified field names
film42 Mar 18, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 11 additions & 8 deletions lib/protobuf/field/base_field.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -177,16 +177,19 @@ def fully_qualified_name_only!
#

def define_accessor(simple_field_name, fully_qualified_field_name)
message_class.class_eval do
define_method("#{simple_field_name}!") do
@values[fully_qualified_field_name]
message_class.class_eval <<-ruby, __FILE__, __LINE__
def #{simple_field_name}!
@values[:"#{fully_qualified_field_name}"]
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I could also do @values[#{fully_qualified_field_name.inspect}], but I don't really have a preference.

end
end

message_class.class_eval do
define_method(simple_field_name) { self[fully_qualified_field_name] }
define_method("#{simple_field_name}=") { |v| self[fully_qualified_field_name] = v }
end
def #{simple_field_name}
self[:"#{fully_qualified_field_name}"]
end

def #{simple_field_name}=(value)
self[:"#{fully_qualified_field_name}"] = value
end
ruby

return unless deprecated?

Expand Down
2 changes: 1 addition & 1 deletion lib/protobuf/field/integer_field.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ class IntegerField < VarintField
#

def decode(value)
value -= 0x1_0000_0000_0000_0000 if (value & 0x8000_0000_0000_0000).nonzero?
value -= 0x1_0000_0000_0000_0000 if (value & 0x8000_0000_0000_0000) != 0
value
end

Expand Down
2 changes: 1 addition & 1 deletion lib/protobuf/field/sfixed32_field.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ class Sfixed32Field < Int32Field

def decode(bytes)
value = bytes.unpack('V').first
value -= 0x1_0000_0000 if (value & 0x8000_0000).nonzero?
value -= 0x1_0000_0000 if (value & 0x8000_0000) != 0
value
end

Expand Down
2 changes: 1 addition & 1 deletion lib/protobuf/field/sfixed64_field.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ class Sfixed64Field < Int64Field
def decode(bytes)
values = bytes.unpack('VV') # 'Q' is machine-dependent, don't use
value = values[0] + (values[1] << 32)
value -= 0x1_0000_0000_0000_0000 if (value & 0x8000_0000_0000_0000).nonzero?
value -= 0x1_0000_0000_0000_0000 if (value & 0x8000_0000_0000_0000) != 0
value
end

Expand Down
2 changes: 1 addition & 1 deletion lib/protobuf/field/signed_integer_field.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ class SignedIntegerField < VarintField
#

def decode(value)
if (value & 1).zero?
if (value & 1) == 0
value >> 1 # positive value
else
~value >> 1 # negative value
Expand Down
8 changes: 4 additions & 4 deletions lib/protobuf/varint_pure.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
module Protobuf
module VarintPure
def decode(stream)
value = index = 0
value = shift = 0
begin
byte = stream.readbyte
value |= (byte & 0x7f) << (7 * index)
index += 1
end while (byte & 0x80).nonzero?
value |= (byte & 127) << shift
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Curious, is the hex actually slower than the dec?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They're about the same, but hex is slower:

irb(main):002:0> Benchmark.ips do |x|
irb(main):003:1* x.report("hex") { 3 + 0x7F }
irb(main):004:1> x.report("dec") { 3 + 127 }
irb(main):005:1> end

Calculating -------------------------------------
                 hex   130.033k i/100ms
                 dec   119.055k i/100ms
-------------------------------------------------
                 hex      8.761M (± 4.6%) i/s -     43.691M
                 dec      9.034M (± 5.7%) i/s -     45.003M

If you think it reads better as a hex (fine either way for me; google uses both), I'll change it back.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems pretty variable:

Warming up --------------------------------------
                 hex   186.594k i/100ms
                 dec   195.158k i/100ms
Calculating -------------------------------------
                 hex      9.104M (± 6.8%) i/s -     45.342M
                 dec      8.828M (± 6.4%) i/s -     44.106M

But I don't want to nitpick, either way is fine :). Hex is nicer for visualizing bitwise arithmetic, but I'm was mostly just curious :)

shift += 7
end while byte > 127
value
end
end
Expand Down
1 change: 1 addition & 0 deletions protobuf.gemspec
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ require "protobuf/version"
s.add_dependency 'thor'
s.add_dependency 'thread_safe'

s.add_development_dependency 'benchmark-ips'
s.add_development_dependency 'ffi-rzmq'
s.add_development_dependency 'rake'
s.add_development_dependency 'rspec', '>= 3.0'
Expand Down