From 659f95748a115a360a37db9bf80a5361a560e14f Mon Sep 17 00:00:00 2001 From: David Heinemeier Hansson Date: Sun, 4 Oct 2026 12:36:36 -0400 Subject: [PATCH] Preload only uncached messages and reduce rendering overhead (#292) * Preload only uncached messages and reduce rendering overhead * Keep benchmark summaries without raw JSON results * Use Ruby benchmark drivers and keep generated results out of the repo --- app/controllers/messages_controller.rb | 6 +- app/controllers/rooms/refreshes_controller.rb | 4 +- app/controllers/rooms_controller.rb | 4 +- app/controllers/searches_controller.rb | 2 +- app/controllers/users/sidebars_controller.rb | 9 +- app/models/message.rb | 6 +- app/models/message/broadcasts.rb | 4 +- app/models/message/pagination.rb | 32 +++++- app/views/searches/index.html.erb | 2 +- bench/README.md | 26 +++++ bench/compare_http.rb | 74 +++++++++++++ bench/compare_message_hot_paths.rb | 61 +++++++++++ bench/http_client.rb | 102 ++++++++++++++++++ bench/message_hot_paths.rb | 92 ++++++++++++++++ bench/support.rb | 84 +++++++++++++++ test/controllers/messages_caching_test.rb | 48 +++++++++ test/models/message_test.rb | 40 ++++++- 17 files changed, 572 insertions(+), 24 deletions(-) create mode 100644 bench/README.md create mode 100644 bench/compare_http.rb create mode 100644 bench/compare_message_hot_paths.rb create mode 100644 bench/http_client.rb create mode 100644 bench/message_hot_paths.rb create mode 100644 bench/support.rb create mode 100644 test/controllers/messages_caching_test.rb diff --git a/app/controllers/messages_controller.rb b/app/controllers/messages_controller.rb index 4cbc771..dae4fd5 100644 --- a/app/controllers/messages_controller.rb +++ b/app/controllers/messages_controller.rb @@ -62,11 +62,11 @@ class MessagesController < ApplicationController def find_paged_messages case when params[:before].present? - @room.messages.with_creator.page_before(@room.messages.find(params[:before])) + @room.messages.with_presentation.page_before(@room.messages.find(params[:before])) when params[:after].present? - @room.messages.with_creator.page_after(@room.messages.find(params[:after])) + @room.messages.with_presentation.page_after(@room.messages.find(params[:after])) else - @room.messages.with_creator.last_page + @room.messages.with_presentation.last_page end end diff --git a/app/controllers/rooms/refreshes_controller.rb b/app/controllers/rooms/refreshes_controller.rb index 05bf119..2c2171e 100644 --- a/app/controllers/rooms/refreshes_controller.rb +++ b/app/controllers/rooms/refreshes_controller.rb @@ -4,8 +4,8 @@ class Rooms::RefreshesController < ApplicationController before_action :set_last_updated_at def show - @new_messages = @room.messages.with_creator.page_created_since(@last_updated_at) - @updated_messages = @room.messages.without(@new_messages).with_creator.page_updated_since(@last_updated_at) + @new_messages = @room.messages.with_presentation.page_created_since(@last_updated_at) + @updated_messages = @room.messages.without(@new_messages).with_presentation.page_updated_since(@last_updated_at) end private diff --git a/app/controllers/rooms_controller.rb b/app/controllers/rooms_controller.rb index 1f1d28e..276265a 100644 --- a/app/controllers/rooms_controller.rb +++ b/app/controllers/rooms_controller.rb @@ -44,9 +44,9 @@ class RoomsController < ApplicationController end def find_messages - messages = @room.messages.with_creator.with_attachment_details.with_boosts + messages = @room.messages.with_presentation - if show_first_message = messages.find_by(id: params[:message_id]) + if params[:message_id].present? && (show_first_message = messages.find_by(id: params[:message_id])) @messages = messages.page_around(show_first_message) else @messages = messages.last_page diff --git a/app/controllers/searches_controller.rb b/app/controllers/searches_controller.rb index 3a2a456..221218d 100644 --- a/app/controllers/searches_controller.rb +++ b/app/controllers/searches_controller.rb @@ -20,7 +20,7 @@ class SearchesController < ApplicationController private def set_messages if query.present? - @messages = Current.user.reachable_messages.search(query).last(100) + @messages = Current.user.reachable_messages.search(query).with_presentation.last_page_of(100) else @messages = Message.none end diff --git a/app/controllers/users/sidebars_controller.rb b/app/controllers/users/sidebars_controller.rb index 2af8050..ce998e8 100644 --- a/app/controllers/users/sidebars_controller.rb +++ b/app/controllers/users/sidebars_controller.rb @@ -2,18 +2,13 @@ class Users::SidebarsController < ApplicationController DIRECT_PLACEHOLDERS = 20 def show - all_memberships = Current.user.memberships.visible.with_ordered_room - @direct_memberships = extract_direct_memberships(all_memberships) - @other_memberships = all_memberships.without(@direct_memberships) + @direct_memberships, @other_memberships = Current.user.memberships.visible.with_ordered_room.partition { |membership| membership.room.direct? } + @direct_memberships = @direct_memberships.sort_by { |membership| membership.room.updated_at }.reverse @direct_placeholder_users = find_direct_placeholder_users end private - def extract_direct_memberships(all_memberships) - all_memberships.select { |m| m.room.direct? }.sort_by { |m| m.room.updated_at }.reverse - end - def find_direct_placeholder_users exclude_user_ids = user_ids_already_in_direct_rooms_with_current_user.including(Current.user.id) User.active.where.not(id: exclude_user_ids).order(:created_at).limit([ DIRECT_PLACEHOLDERS - exclude_user_ids.count, 0 ].max) diff --git a/app/models/message.rb b/app/models/message.rb index e607f6a..0c4c1b9 100644 --- a/app/models/message.rb +++ b/app/models/message.rb @@ -14,11 +14,11 @@ class Message < ApplicationRecord scope :ordered, -> { order(:created_at) } scope :with_creator, -> { preload(creator: :avatar_attachment) } scope :with_attachment_details, -> { - with_rich_text_body_and_embeds - with_attached_attachment + with_rich_text_body_and_embeds.with_attached_attachment .includes(attachment_blob: :variant_records) } - scope :with_boosts, -> { includes(boosts: :booster) } + scope :with_boosts, -> { includes(boosts: { booster: :avatar_attachment }) } + scope :with_presentation, -> { with_creator.with_attachment_details.with_boosts.preload(:room) } def plain_text_body body.to_plain_text.presence || attachment&.filename&.to_s || "" diff --git a/app/models/message/broadcasts.rb b/app/models/message/broadcasts.rb index 4623f2a..f82e4e3 100644 --- a/app/models/message/broadcasts.rb +++ b/app/models/message/broadcasts.rb @@ -12,8 +12,10 @@ module Message::Broadcasts # Fanned out to the room's members rather than published on one global stream, so # that the timing of activity in a room only reaches people who are in it. def broadcast_unread_room + payload = ActiveSupport::JSON.encode(roomId: room.id) + room.memberships.pluck(:user_id).each do |user_id| - ActionCable.server.broadcast UnreadRoomsChannel.stream_name_for(user_id), { roomId: room.id } + ActionCable.server.broadcast UnreadRoomsChannel.stream_name_for(user_id), payload, coder: nil end end end diff --git a/app/models/message/pagination.rb b/app/models/message/pagination.rb index c16c3e6..3be7983 100644 --- a/app/models/message/pagination.rb +++ b/app/models/message/pagination.rb @@ -3,9 +3,31 @@ module Message::Pagination PAGE_SIZE = 40 + # Expose Rails' collection-preloading hook while preserving Array pagination and + # validators. The renderer passes only cache misses to preload_associations. + class Page < Array + def self.load(relation, direction, size) + new(relation.skip_preloading!.public_send(direction, size), relation) + end + + def initialize(records, relation) + super(records) + @relation = relation + end + + def loaded? + true + end + + def preload_associations(records) + @relation.preload_associations(records) + end + end + included do - scope :last_page, -> { ordered.last(PAGE_SIZE) } - scope :first_page, -> { ordered.first(PAGE_SIZE) } + # Keep presentation data lazy until the collection cache knows which messages missed. + scope :last_page, -> { last_page_of(PAGE_SIZE) } + scope :first_page, -> { Page.load(ordered, :first, PAGE_SIZE) } scope :before, ->(message) { where("created_at < ?", message.created_at) } scope :after, ->(message) { where("created_at > ?", message.created_at) } @@ -18,8 +40,12 @@ module Message::Pagination end class_methods do + def last_page_of(size) + Page.load(ordered, :last, size) + end + def page_around(message) - page_before(message) + [ message ] + page_after(message) + Page.new(page_before(message) + [ message ] + page_after(message), ordered) end def paged? diff --git a/app/views/searches/index.html.erb b/app/views/searches/index.html.erb index 4b78269..cbaa476 100644 --- a/app/views/searches/index.html.erb +++ b/app/views/searches/index.html.erb @@ -54,7 +54,7 @@ <%= search_results_tag do %> - <%= render @messages %> + <%= render partial: "messages/message", collection: @messages, cached: true %> <% end %> diff --git a/bench/README.md b/bench/README.md new file mode 100644 index 0000000..ca5cd5e --- /dev/null +++ b/bench/README.md @@ -0,0 +1,26 @@ +# Message benchmarks + +The drivers use Ruby's standard library and Docker; no separate load generator is required. +Provide a frozen baseline source directory, an isolated seed containing `db/`, `storage/` +and `labels.json`, and a matching application image with compiled assets. + +```sh +ruby bench/compare_message_hot_paths.rb --baseline PATH --baseline-ref SHA --seed SEED +ruby bench/compare_http.rb --baseline PATH --seed SEED +ruby bench/compare_http.rb --baseline PATH --seed SEED --paths sidebar,search --concurrencies 16 --duration 10 +``` + +Both drivers alternate before/after order and reset fixture storage for each run. +Results default to ignored `tmp/rails-optimization/results/`; override with `--output PATH`. +Use `--image` to override `campfire-reference:app`, and `--cpus` to override server CPUs +`8-11`. HTTP clients use CPUs `12-15` by default (`--client-cpus`). `--help` lists options. + +The rendering probe checks exact response bodies, selected headers and unread payloads, +and records timing, queries and allocations with MemoryStore, frozen time and fixture-only +CSRF disabling. Unread fanout excludes adapter I/O. + +The HTTP driver uses production Puma/Redis with one worker and five threads. Ruby threads +each maintain a keep-alive connection, request uncompressed responses, and consume the +whole body. Login uses normal CSRF protection; all warmup and measured responses must be +HTTP 200 without transport errors. Measurements exclude Thruster, TLS and gzip. Client +CPU, JIT warmup and GC can affect throughput; repeat runs and check client saturation. diff --git a/bench/compare_http.rb b/bench/compare_http.rb new file mode 100644 index 0000000..58eb421 --- /dev/null +++ b/bench/compare_http.rb @@ -0,0 +1,74 @@ +#!/usr/bin/env ruby +# Compare warm production Puma/Redis requests using isolated seeded containers. +require "socket" +require_relative "support" +require_relative "http_client" + +include BenchmarkSupport +options = parse_options("Compare HTTP throughput with Ruby keep-alive clients; every response must be HTTP 200.", + duration: 3.0, paths: "room,messages,sidebar,search", concurrencies: "1,16", client_cpus: "12-15", + output: File.join(WORK, "results/http")) +labels = JSON.parse(File.read(File.join(options[:seed], "labels.json"))) +paths = { + "room" => "/rooms/#{labels.fetch('rooms.watercooler')}", + "messages" => "/rooms/#{labels.fetch('rooms.watercooler')}/messages?before=#{labels.fetch('messages.busy_060')}", + "sidebar" => "/users/me/sidebar", "search" => "/searches?q=coffee" +}.slice(*options[:paths].split(",")) +abort "--paths must select room,messages,sidebar,search" unless paths.size == options[:paths].split(",").size && !paths.empty? +concurrencies = options[:concurrencies].split(",").map { |value| Integer(value) } +abort "--duration and --concurrencies must be positive" unless options[:duration].positive? && !concurrencies.empty? && concurrencies.all?(&:positive?) +run("taskset", "-pc", options[:client_cpus], Process.pid.to_s) +work = File.join(WORK, "http") +assets = prepare_assets(options[:image]) +network = "cf-ruby-bench-#{Process.pid}" +redis = "#{network}-redis" +app = "#{network}-app" + +begin + run("docker", "network", "create", network) + run("docker", "run", "-d", "--name", redis, "--network", network, "redis:7-alpine") + options[:rounds].times do |iteration| + sides = iteration.even? ? %w[before after] : %w[after before] + sides.each do |side| + remove_container(app) + data = File.join(work, "data") + prepare_storage(options[:seed], File.join(data, "storage")) + FileUtils.mkdir_p([ File.join(data, "tmp/pids"), File.join(data, "log") ]) + run("docker", "exec", redis, "redis-cli", "FLUSHALL") + port = TCPServer.open("127.0.0.1", 0) { |socket| socket.addr[1] } + client = BenchmarkHTTPClient.new("http://127.0.0.1:#{port}") + source = side == "before" ? options[:baseline] : ROOT + command = [ "docker", "run", "-d", "--name", app, "--entrypoint", "", "--network", network, + "--cpuset-cpus", options[:cpus], "-p", "127.0.0.1:#{port}:3000" ] + command.concat mounts(source => "/rails", File.join(data, "storage") => "/rails/storage", + File.join(data, "tmp") => "/rails/tmp", File.join(data, "log") => "/rails/log", assets => "/rails/public/assets") + command.concat environment(RAILS_ENV: "production", SECRET_KEY_BASE: "isolated-benchmark-fixture-key", DISABLE_SSL: true, + SKIP_TELEMETRY: true, RAILS_LOG_LEVEL: "fatal", WEB_CONCURRENCY: 1, JOB_CONCURRENCY: 1, RAILS_MAX_THREADS: 5, + REDIS_URL: "redis://#{redis}:6379/0") + command.concat [ options[:image], "bundle", "exec", "puma", "-C", "config/puma.rb" ] + run(*command) + deadline = clock + 45 + until client.ready? + if clock > deadline + File.write(File.join(work, "server.log"), run("docker", "logs", app)) + raise "server did not become ready; see #{work}/server.log" + end + sleep 0.1 + end + cookie = client.login(labels) + results = {} + paths.each do |name, path| + client.measure(path, cookie, concurrency: 1, duration: 3) + concurrencies.each do |concurrency| + results["#{name}_#{concurrency}"] = client.measure(path, cookie, concurrency: concurrency, duration: options[:duration]) + end + end + write_json(File.join(options[:output], "#{side}-#{iteration + 1}.json"), results) + puts "#{iteration + 1}/#{options[:rounds]}: #{side}" + end + end +ensure + remove_container(app) + remove_container(redis) + Open3.capture3("docker", "network", "rm", network) +end diff --git a/bench/compare_message_hot_paths.rb b/bench/compare_message_hot_paths.rb new file mode 100644 index 0000000..225ee3e --- /dev/null +++ b/bench/compare_message_hot_paths.rb @@ -0,0 +1,61 @@ +#!/usr/bin/env ruby +# Compare frozen Rails sources with isolated fixtures and an existing Ruby image. +require "digest" +require_relative "support" + +include BenchmarkSupport +options = parse_options("Compare rendering queries, allocations, timing and response parity.", + baseline_ref: nil, rounds: 4, output: File.join(WORK, "results/requests")) +work = File.join(WORK, "runtime") +FileUtils.mkdir_p([ File.join(work, "tmp"), File.join(work, "log") ]) +assets = prepare_assets(options[:image]) +runs = { "before" => [], "after" => [] } +order = [] +options[:rounds].times do |iteration| + sides = iteration.even? ? %w[before after] : %w[after before] + order << sides + sides.each do |side| + storage = File.join(work, "storage") + prepare_storage(options[:seed], storage) + source = side == "before" ? options[:baseline] : ROOT + command = [ "docker", "run", "--rm", "--entrypoint", "", "--cpuset-cpus", options[:cpus] ] + command.concat mounts(source => "/rails", storage => "/rails/storage", File.join(work, "tmp") => "/rails/tmp", + File.join(work, "log") => "/rails/log", assets => "/rails/public/assets", File.join(ROOT, "bench") => "/bench", + File.join(options[:seed], "labels.json") => "/bench-labels.json") + command.concat environment(RAILS_ENV: "production", SECRET_KEY_BASE: "isolated-benchmark-fixture-key", DISABLE_SSL: true, + SKIP_TELEMETRY: true, RAILS_LOG_LEVEL: "fatal", BENCH_LABELS: "/bench-labels.json") + command.concat [ options[:image], "bundle", "exec", "ruby", "-r", "/rails/config/environment.rb", "/bench/message_hot_paths.rb" ] + data = JSON.parse(run(*command)) + runs[side] << data + write_json(File.join(options[:output], "#{side}-#{iteration + 1}.json"), data) + puts "#{iteration + 1}/#{options[:rounds]}: #{side}" + end +end + +summary = {} +runs["before"].first.fetch("results").each_key do |name| + values = runs.transform_values { |rows| rows.map { |row| row.fetch("results").fetch(name) } } + %w[body_sha256 headers payload_sha256].each do |field| + expected = values["before"].first[field] + raise "#{name}: #{field} differs; no winning summary written" unless values.values.flatten.all? { |row| row[field] == expected } + end + summary[name] = values.transform_values do |rows| + fields = %w[milliseconds allocations queries].select { |field| rows.first.key?(field) } + fields.to_h { |field| [ field, median(rows.map { |row| median(row.fetch(field)) }) ] } + .merge("round_medians_ms" => rows.map { |row| median(row.fetch("milliseconds")) }) + end + summary[name]["speedup"] = summary[name]["before"]["milliseconds"] / summary[name]["after"]["milliseconds"] +end +metadata = { + baseline_sha: options[:baseline_ref], candidate_head: run("git", "-C", ROOT, "rev-parse", "HEAD").strip, + candidate_app_diff_sha256: Digest::SHA256.hexdigest(run("git", "-C", ROOT, "diff", "--", "app")), + platform: RUBY_PLATFORM, cpus: options[:cpus], image: options[:image], + image_id: run("docker", "image", "inspect", "-f", "{{.Id}}", options[:image]).strip, + body_and_selected_header_parity: "exact across all measured baseline/candidate requests", + order: order, ruby: runs["before"].first.fetch("ruby"), rails: runs["before"].first.fetch("rails"), + limits: "In-process Rails requests, MemoryStore, frozen clock and fixture-controller CSRF disabled. Fanout excludes adapter I/O. Not network throughput or connection capacity." +} +write_json(File.join(options[:output], "summary.json"), metadata: metadata, results: summary) +summary.each do |name, value| + puts "%s: %.2f -> %.2f ms (%.2fx)" % [ name, value["before"]["milliseconds"], value["after"]["milliseconds"], value["speedup"] ] +end diff --git a/bench/http_client.rb b/bench/http_client.rb new file mode 100644 index 0000000..574f3b7 --- /dev/null +++ b/bench/http_client.rb @@ -0,0 +1,102 @@ +require "cgi" +require "net/http" + +class BenchmarkHTTPClient + def initialize(base) + @base = URI(base) + end + + def ready? + connection.start { |http| http.get("/up").code == "200" } + rescue IOError, SystemCallError, Timeout::Error, SocketError + false + end + + def login(labels) + cookies = {} + connection.start do |http| + response = http.get("/session/new", "Accept-Encoding" => "identity") + raise "sign-in page: HTTP #{response.code}" unless response.code == "200" + merge_cookies(cookies, response) + token = response.body[/ cookie, "Accept-Encoding" => "identity") + result[:latencies] << (clock - requested) * 1000 + result[:statuses][response.code] += 1 + result[:bytes] += response.body.bytesize + end + end + rescue IOError, SystemCallError, Timeout::Error, SocketError, Net::HTTPBadResponse + result[:errors] += 1 + sleep 0.01 + end + end + result + end + end + samples = workers.map(&:value) + elapsed = clock - start + latencies = samples.flat_map { |sample| sample[:latencies] }.sort + statuses = Hash.new(0) + samples.each { |sample| sample[:statuses].each { |status, count| statuses[status] += count } } + errors = samples.sum { |sample| sample[:errors] } + raise "#{path}: HTTP statuses #{statuses}, #{errors} transport errors" unless errors.zero? && statuses.keys == [ "200" ] + { path: path, conc: concurrency, gzip: false, secs: elapsed, rps: latencies.size / elapsed, + ok: latencies.size, statuses: statuses, errors: errors, + avg_bytes: samples.sum { |sample| sample[:bytes] } / latencies.size, + latency_ms: { p50: percentile(latencies, 0.50), p95: percentile(latencies, 0.95), p99: percentile(latencies, 0.99) } } + end + + private + def connection + Net::HTTP.new(@base.host, @base.port, nil).tap do |http| + http.open_timeout = 1 + http.read_timeout = 5 + http.write_timeout = 5 + http.max_retries = 0 + end + end + + def merge_cookies(cookies, response) + response.get_fields("set-cookie").to_a.each do |header| + name, value = header.split(";", 2).first.split("=", 2) + cookies[name] = value + end + end + + def cookie_header(cookies) + cookies.map { |name, value| "#{name}=#{value}" }.join("; ") + end + + def percentile(values, fraction) + values[(values.size * fraction).ceil - 1] + end + + def clock + Process.clock_gettime(Process::CLOCK_MONOTONIC) + end +end diff --git a/bench/message_hot_paths.rb b/bench/message_hot_paths.rb new file mode 100644 index 0000000..4e5ba37 --- /dev/null +++ b/bench/message_hot_paths.rb @@ -0,0 +1,92 @@ +# Run through compare_message_hot_paths.rb against isolated production fixtures. +require "json" +require "digest" +require "active_support/testing/time_helpers" + +include ActiveSupport::Testing::TimeHelpers +travel_to Time.utc(2026, 10, 4, 12) + +# Keep framework/rendering measurements independent of Redis/network variance. +Rails.cache = ActiveSupport::Cache::MemoryStore.new +ActionView::PartialRenderer.collection_cache = Rails.cache +ApplicationController.cache_store = Rails.cache +ApplicationController.descendants.each do |controller| + controller.cache_store = Rails.cache + controller.allow_forgery_protection = false +end +ApplicationController.allow_forgery_protection = false + +labels = JSON.parse(File.read(ENV.fetch("BENCH_LABELS"))) +client = ActionDispatch::Integration::Session.new(Rails.application) +client.host! "campfire-benchmark.test" +client.post "/session", params: { email_address: labels.fetch("emails.david"), password: labels.fetch("passwords.all") } +raise "login failed" unless client.response.redirect? + +paths = { + room: "/rooms/#{labels.fetch('rooms.watercooler')}", + messages: "/rooms/#{labels.fetch('rooms.watercooler')}/messages?before=#{labels.fetch('messages.busy_060')}", + sidebar: Rails.application.routes.url_helpers.user_sidebar_path, + search: "/searches?q=coffee" +} +queries = [] +subscriber = ActiveSupport::Notifications.subscribe("sql.active_record") do |*, event| + queries << event[:sql] unless event[:cached] || event[:name] == "SCHEMA" +end +results = {} +paths.each do |name, path| + %w[cold warm].each do |cache| + 2.times { client.get path } + times, counts, allocations, hashes, headers = [], [], [], [], [] + Integer(ENV.fetch("BENCH_ITERATIONS", "20")).times do + Rails.cache.clear if cache == "cold" + queries.clear + start_allocations = GC.stat(:total_allocated_objects) + start = Process.clock_gettime(Process::CLOCK_MONOTONIC) + client.get path + times << (Process.clock_gettime(Process::CLOCK_MONOTONIC) - start) * 1000 + allocations << GC.stat(:total_allocated_objects) - start_allocations + raise "#{name}: HTTP #{client.response.status}" unless client.response.status == 200 + counts << queries.length + hashes << Digest::SHA256.hexdigest(client.response.body) + headers << client.response.headers.slice("content-type", "etag", "cache-control", "vary") + end + results["#{name}_#{cache}"] = { milliseconds: times, queries: counts, allocations: allocations, + body_sha256: hashes.uniq, headers: headers.uniq, body_bytes: client.response.body.bytesize } + end +end +ActiveSupport::Notifications.unsubscribe(subscriber) + +# Real Action Cable encoding/instrumentation, with adapter I/O removed. Keep fanout +# separate from network delivery capacity, which this probe does not measure. +room = Room.find(labels.fetch("rooms.watercooler")) +class BenchmarkPubsub + attr_reader :messages + def initialize + @messages = [] + end + def broadcast(stream, payload) + @messages << [ stream, payload ] + end +end +adapter = BenchmarkPubsub.new +ActionCable.server.instance_variable_set(:@pubsub, adapter) +message = Message.new(room: room) +recipient_ids = (1..1000).to_a +memberships = Object.new +memberships.define_singleton_method(:pluck) { |_| recipient_ids } +room.define_singleton_method(:memberships) { memberships } +fanout_times, fanout_allocations = [], [] +20.times do + adapter.messages.clear + allocated = GC.stat(:total_allocated_objects) + start = Process.clock_gettime(Process::CLOCK_MONOTONIC) + message.send(:broadcast_unread_room) + fanout_times << (Process.clock_gettime(Process::CLOCK_MONOTONIC) - start) * 1000 + fanout_allocations << GC.stat(:total_allocated_objects) - allocated + raise "wrong fanout count" unless adapter.messages.size == recipient_ids.size + raise "wrong payload" unless adapter.messages.all? { |_, body| JSON.parse(body) == { "roomId" => room.id } } +end +results["unread_fanout_1000"] = { milliseconds: fanout_times, allocations: fanout_allocations, + payload_sha256: Digest::SHA256.hexdigest(adapter.messages.first.last) } +puts JSON.pretty_generate({ ruby: RUBY_VERSION, rails: Rails.version, iterations: ENV.fetch("BENCH_ITERATIONS", "20").to_i, + cache: "MemoryStore", results: results }) diff --git a/bench/support.rb b/bench/support.rb new file mode 100644 index 0000000..c0c4089 --- /dev/null +++ b/bench/support.rb @@ -0,0 +1,84 @@ +require "fileutils" +require "json" +require "open3" +require "optparse" + +module BenchmarkSupport + ROOT = File.expand_path("..", __dir__) + WORK = File.join(ROOT, "tmp/rails-optimization") + + def parse_options(description, defaults) + options = { baseline: nil, seed: nil, image: "campfire-reference:app", cpus: "8-11", rounds: 2 }.merge(defaults) + parser = OptionParser.new do |parser| + parser.banner = "Usage: ruby #{$PROGRAM_NAME} --baseline PATH --seed PATH [options]\n#{description}" + options.each do |name, default| + type = case default + when Integer then Integer + when Float then Float + else String + end + parser.on("--#{name.to_s.tr('_', '-')} VALUE", type) { |value| options[name] = value } + end + parser.on("-h", "--help") { puts parser; exit } + end + parser.parse! + raise OptionParser::InvalidArgument, "unexpected arguments: #{ARGV.join(' ')}" unless ARGV.empty? + raise OptionParser::MissingArgument, "--baseline and --seed are required" unless options[:baseline] && options[:seed] + raise OptionParser::InvalidArgument, "--rounds must be even and at least 2" unless options[:rounds] >= 2 && options[:rounds].even? + %i[baseline seed output].each { |name| options[name] = File.expand_path(options.fetch(name)) } + options + rescue OptionParser::ParseError => error + abort "#{error.message}\n#{parser}" + end + + def run(*command, input: nil) + output, errors, status = Open3.capture3(*command, stdin_data: input, binmode: true) + raise "#{command.first} failed (#{status.exitstatus}): #{errors}" unless status.success? + output + end + + def remove_container(name) + Open3.capture3("docker", "rm", "-f", name) + end + + def prepare_storage(seed, destination) + FileUtils.rm_rf(destination) + FileUtils.mkdir_p(destination) + FileUtils.cp_r(File.join(seed, "db"), File.join(destination, "db")) + FileUtils.cp_r(File.join(seed, "storage"), File.join(destination, "files")) + end + + def prepare_assets(image) + assets = File.join(WORK, "runtime/assets") + FileUtils.mkdir_p(assets) + if Dir.empty?(assets) + archive = run("docker", "run", "--rm", "--entrypoint", "", image, + "tar", "-C", "/rails/public/assets", "-cf", "-", ".") + run("tar", "-xf", "-", "-C", assets, input: archive) + end + assets + end + + def mounts(paths) + paths.flat_map { |host, target| [ "-v", "#{host}:#{target}" ] } + end + + def environment(values) + values.flat_map { |name, value| [ "-e", "#{name}=#{value}" ] } + end + + def write_json(path, data) + FileUtils.mkdir_p(File.dirname(path)) + File.write(path, JSON.pretty_generate(data) + "\n") + end + + def median(values) + values = values.sort + middle = values.length / 2 + values.length.odd? ? values[middle] : (values[middle - 1] + values[middle]) / 2.0 + end + + def clock + Process.clock_gettime(Process::CLOCK_MONOTONIC) + end +end diff --git a/test/controllers/messages_caching_test.rb b/test/controllers/messages_caching_test.rb new file mode 100644 index 0000000..131a078 --- /dev/null +++ b/test/controllers/messages_caching_test.rb @@ -0,0 +1,48 @@ +require "test_helper" +require "active_record/testing/query_assertions" + +class MessagesCachingTest < ActionDispatch::IntegrationTest + include ActiveRecord::Assertions::QueryAssertions + + setup do + sign_in :david + end + + test "cached pages skip presentation queries and refresh edited messages" do + with_memory_cache do + get room_messages_url(rooms(:watercooler)) + assert_response :success + original = response.body + + assert_no_queries_match(/action_text_rich_texts|active_storage_attachments|boosts/) do + get room_messages_url(rooms(:watercooler)) + end + assert_response :success + assert_equal original, response.body + + messages(:fourth).update! body: "Updated cached message" + get room_messages_url(rooms(:watercooler)) + assert_response :success + assert_select "#" + dom_id(messages(:fourth)), text: /Updated cached message/ + end + end + + private + def with_memory_cache + old_cache = Rails.cache + old_collection_cache = ActionView::PartialRenderer.collection_cache + old_controller_cache = MessagesController.cache_store + old_caching = MessagesController.perform_caching + + Rails.cache = ActiveSupport::Cache::MemoryStore.new + ActionView::PartialRenderer.collection_cache = Rails.cache + MessagesController.cache_store = Rails.cache + MessagesController.perform_caching = true + yield + ensure + Rails.cache = old_cache + ActionView::PartialRenderer.collection_cache = old_collection_cache + MessagesController.cache_store = old_controller_cache + MessagesController.perform_caching = old_caching + end +end diff --git a/test/models/message_test.rb b/test/models/message_test.rb index 3cd12c1..5ffecea 100644 --- a/test/models/message_test.rb +++ b/test/models/message_test.rb @@ -1,7 +1,8 @@ require "test_helper" +require "active_record/testing/query_assertions" class MessageTest < ActiveSupport::TestCase - include ActionCable::TestHelper, ActiveJob::TestHelper + include ActionCable::TestHelper, ActiveJob::TestHelper, ActiveRecord::Assertions::QueryAssertions test "creating a message enqueues to push later" do assert_enqueued_jobs 1, only: [ Room::PushMessageJob ] do @@ -27,6 +28,43 @@ class MessageTest < ActiveSupport::TestCase assert_equal [], message_mentioning_a_non_member.mentionees end + test "presentation associations load together" do + messages(:first).attachment.attach io: StringIO.new("hello"), filename: "hello.txt", content_type: "text/plain" + presented = Message.where(id: [ messages(:first).id, messages(:thirteenth).id ]).with_presentation.to_a + + assert_no_queries do + presented.each do |message| + message.room.name + message.creator.avatar.attached? + message.body.body.to_html + message.body.embeds.each(&:filename) + message.attachment.blob&.filename + message.boosts.each { |boost| boost.booster.avatar.attached? } + end + end + end + + test "presentation pages preload only the rendered messages" do + page = rooms(:watercooler).messages.with_presentation.last_page + assert_kind_of Array, page + assert page.all? { |message| !message.association(:rich_text_body).loaded? } + + rendered = page.first(2) + page.preload_associations(rendered) + + assert_no_queries { rendered.each { |message| message.body.to_plain_text } } + assert page.drop(2).all? { |message| !message.association(:rich_text_body).loaded? } + end + + test "pagination retains the original ordering even when timestamps tie" do + room = rooms(:watercooler) + room.messages.update_all(created_at: Time.current) + expected = room.messages.ordered.last(Message::Pagination::PAGE_SIZE).map(&:id) + + assert_equal expected, room.messages.last_page.map(&:id) + assert_equal room.messages.ordered.first(Message::Pagination::PAGE_SIZE).map(&:id), room.messages.first_page.map(&:id) + end + private def create_new_message_in(room) room.messages.create!(creator: users(:jason), body: "Hello", client_message_id: "123")