Skip to content

Proof of concept for ActiveSupport::Notifications #61

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.

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 7 commits into from
Closed
Show file tree
Hide file tree
Changes from 6 commits
Commits
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
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ $ bundle install

```ruby
class Movie
attr_accessor :id, :name, :year, :actor_ids, :owner_id
attr_accessor :id, :name, :year, :actor_ids, :owner_id, :movie_type_id
end
```

Expand Down
59 changes: 41 additions & 18 deletions lib/fast_jsonapi/object_serializer.rb
Original file line number Diff line number Diff line change
@@ -1,36 +1,45 @@
require 'active_support/core_ext/object'
require 'active_support/concern'
require 'active_support/inflector'
require 'active_support/notifications'
require 'oj'
require 'multi_json'
require 'fast_jsonapi/serialization_core'

begin
require 'skylight'
SKYLIGHT_ENABLED = true
rescue LoadError
SKYLIGHT_ENABLED = false
end

module FastJsonapi
module ObjectSerializer

if ENV['SERIALIZATION_INSTRUMENTATION_ENABLED'.freeze] == 'true'.freeze
INSTRUMENTATION_ENABLED = true
elsif ENV['SERIALIZATION_INSTRUMENTATION_DISABLED'.freeze] == 'true'.freeze
INSTRUMENTATION_ENABLED = false
else
INSTRUMENTATION_ENABLED = false
end

SERIALIZE_HASH_NOTIFICATION = 'render.fast_jsonapi.serializable_hash'.freeze
TO_JSON_HASH_NOTIFICATION = 'render.fast_jsonapi.to_json'.freeze

extend ActiveSupport::Concern
include SerializationCore

included do
# Skylight integration
# To remove Skylight
# Remove the included do block
# Remove the Gemfile entry
if SKYLIGHT_ENABLED
include Skylight::Helpers

instrument_method :serializable_hash
instrument_method :to_json
end

# Set record_type based on the name of the serializer class
set_type default_record_type if default_record_type

self.class_eval do
alias_method :to_json_without_instrumentation, :to_json

def to_json
Copy link
Collaborator

Choose a reason for hiding this comment

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

we dont have a method called to_json. I think the method we use is called serialized_json to get the JSON string representation.

I like the simple and readable approach you have taken with serializable_hash can we do the same with serialized_json?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I was going off the original Skylight instrumenting, here, which did serializable_hash and to_json. It would be easy enough to change.

if instrumentation_enabled?
ActiveSupport::Notifications.instrument(TO_JSON_HASH_NOTIFICATION, { name: self.class.name }) do
to_json_without_instrumentation
end
else
to_json_without_instrumentation
end
end
end
end

def initialize(resource, options = {})
Expand All @@ -48,7 +57,21 @@ def initialize(resource, options = {})
end
end

def instrumentation_enabled?
INSTRUMENTATION_ENABLED
end

def serializable_hash
if instrumentation_enabled?
ActiveSupport::Notifications.instrument(SERIALIZE_HASH_NOTIFICATION, { name: self.class.name }) do
serializable_hash!
end
else
serializable_hash!
end
end

def serializable_hash!
serializable_hash = { data: nil }
serializable_hash[:meta] = @meta_tags if @meta_tags.present?
return hash_for_one_record(serializable_hash) if @record
Expand Down
59 changes: 59 additions & 0 deletions spec/lib/object_serializer_notifications_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
require 'spec_helper'

describe FastJsonapi::ObjectSerializer do
include_context 'movie class'

context 'instrument' do
Copy link
Collaborator

Choose a reason for hiding this comment

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

it would be more readable if the spec were named a little elaborately. It's important to test both cases.

context 'when calling serializable_hash' do
  it 'should send a notification' do
  end
  
  it  'shouldnt send a notification' do
  end
end

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Of course. These tests were mostly just to make sure that I was actually getting notifications. :p

TDD FTW!!!

it 'serializable_hash' do
options = {}
options[:meta] = { total: 2 }
options[:include] = [:actors]

serializer = MovieSerializer.new([movie, movie], options)
allow(serializer).to receive(:instrumentation_enabled?).and_return(true)

events = []

ActiveSupport::Notifications.subscribe(FastJsonapi::ObjectSerializer::SERIALIZE_HASH_NOTIFICATION) do |*args|
events << ActiveSupport::Notifications::Event.new(*args)
end

serialized_hash = serializer.serializable_hash

event = events.first

expect(event.duration).to be > 0
expect(event.payload).to eq({ name: 'MovieSerializer' })
expect(event.name).to eq(FastJsonapi::ObjectSerializer::SERIALIZE_HASH_NOTIFICATION)

expect(serialized_hash.key?(:data)).to eq(true)
expect(serialized_hash.key?(:meta)).to eq(true)
expect(serialized_hash.key?(:included)).to eq(true)
end

it 'to_json' do
options = {}
options[:meta] = { total: 2 }
options[:include] = [:actors]

serializer = MovieSerializer.new([movie, movie], options)
allow(serializer).to receive(:instrumentation_enabled?).and_return(true)

events = []

ActiveSupport::Notifications.subscribe(FastJsonapi::ObjectSerializer::TO_JSON_HASH_NOTIFICATION) do |*args|
events << ActiveSupport::Notifications::Event.new(*args)
end

json = serializer.to_json

event = events.first

expect(event.duration).to be > 0
expect(event.payload).to eq({ name: 'MovieSerializer' })
expect(event.name).to eq(FastJsonapi::ObjectSerializer::TO_JSON_HASH_NOTIFICATION)

expect(json.length).to be > 50
end
end
end
49 changes: 36 additions & 13 deletions spec/lib/object_serializer_performance_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,9 @@
include_context 'movie class'
include_context 'ams movie class'

before(:all) { GC.disable }
after(:all) { GC.enable }

context 'when testing performance of serialization' do
it 'should create a hash of 1000 records in less than 50 ms' do
movies = 1000.times.map { |_i| movie }
Expand Down Expand Up @@ -34,29 +37,48 @@
end
end

def print_stats(count, ams_time, our_time)
def print_stats(message, count, ams_time, our_time)
format = '%-15s %-10s %s'
puts ''
puts message
puts format(format, 'Serializer', 'Records', 'Time')
puts format(format, 'AMS serializer', count, ams_time.round(2).to_s + ' ms')
puts format(format, 'Fast serializer', count, our_time.round(2).to_s + ' ms')
end

def run_hash_benchmark(message, movie_count, our_serializer, ams_serializer)
our_time = Benchmark.measure { our_hash = our_serializer.serializable_hash }.real * 1000
ams_time = Benchmark.measure { ams_hash = ams_serializer.as_json }.real * 1000
print_stats(message, movie_count, ams_time, our_time)
end

def run_json_benchmark(message, movie_count, our_serializer, ams_serializer)
our_json = nil
ams_json = nil
our_time = Benchmark.measure { our_json = our_serializer.serialized_json }.real * 1000
ams_time = Benchmark.measure { ams_json = ams_serializer.to_json }.real * 1000
print_stats(message, movie_count, ams_time, our_time)
return our_json, ams_json
end

context 'when comparing with AMS 0.10.x' do
[1, 25, 250, 1000].each do |movie_count|
speed_factor = 25
it "should serialize #{movie_count} records atleast #{speed_factor} times faster than AMS" do
ams_movies = build_ams_movies(movie_count)
movies = build_movies(movie_count)
our_json = nil
ams_json = nil
our_serializer = MovieSerializer.new(movies)
ams_serializer = ActiveModelSerializers::SerializableResource.new(ams_movies)
our_time = Benchmark.measure { our_json = our_serializer.serialized_json }.real * 1000
ams_time = Benchmark.measure { ams_json = ams_serializer.to_json }.real * 1000
print_stats(movie_count, ams_time, our_time)

message = "Serialize to JSON string #{movie_count} records"
our_json, ams_json = run_json_benchmark(message, movie_count, our_serializer, ams_serializer)

message = "Serialize to Ruby Hash #{movie_count} records"
run_hash_benchmark(message, movie_count, our_serializer, ams_serializer)

expect(our_json.length).to eq ams_json.length
expect { our_serializer.serialized_json }.to perform_faster_than { ams_serializer.to_json }.at_least(speed_factor).times
expect { our_serializer.serializable_hash }.to perform_faster_than { ams_serializer.as_json }.at_least(speed_factor).times
end
end
end
Expand All @@ -67,20 +89,21 @@ def print_stats(count, ams_time, our_time)
it "should serialize #{movie_count} records atleast #{speed_factor} times faster than AMS" do
ams_movies = build_ams_movies(movie_count)
movies = build_movies(movie_count)
our_json = nil
ams_json = nil

options = {}
options[:meta] = { total: movie_count }
options[:include] = [:actors, :movie_type]

our_serializer = MovieSerializer.new(movies, options)
ams_serializer = ActiveModelSerializers::SerializableResource.new(ams_movies, include: options[:include], meta: options[:meta])
our_time = Benchmark.measure { our_json = our_serializer.serialized_json }.real * 1000
ams_time = Benchmark.measure { ams_json = ams_serializer.to_json }.real * 1000
print_stats(movie_count, ams_time, our_time)

message = "Serialize to JSON string #{movie_count} with includes and meta"
our_json, ams_json = run_json_benchmark(message, movie_count, our_serializer, ams_serializer)

message = "Serialize to Ruby Hash #{movie_count} with includes and meta"
run_hash_benchmark(message, movie_count, our_serializer, ams_serializer)

expect(our_json.length).to eq ams_json.length
expect { our_serializer.serialized_json }.to perform_faster_than { ams_serializer.to_json }.at_least(speed_factor).times
expect { our_serializer.serializable_hash }.to perform_faster_than { ams_serializer.as_json }.at_least(speed_factor).times
end
end
end
Expand Down
2 changes: 2 additions & 0 deletions spec/spec_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
require 'multi_json'
require 'byebug'
require 'active_model_serializers'
require 'oj'

Dir[File.dirname(__FILE__) + '/shared/contexts/*.rb'].each {|file| require file }

Expand All @@ -13,6 +14,7 @@
end
end

Oj.optimize_rails
ActiveModel::Serializer.config.adapter = :json_api
ActiveModel::Serializer.config.key_transform = :underscore
ActiveModelSerializers.logger = ActiveSupport::TaggedLogging.new(ActiveSupport::Logger.new('/dev/null'))