Project

General

Profile

Actions

Bug #18454

open

YJIT slowing down key Discourse benchmarks

Added by sam.saffron (Sam Saffron) 5 months ago. Updated 5 months ago.

Status:
Open
Priority:
Normal
Assignee:
-
Target version:
-
[ruby-core:106928]

Description

4 out of 6 key Discourse benchmarks used in: https://github.com/discourse/discourse/blob/main/script/bench.rb are slower under YJIT.

Vanilla (median) -> YJIT (median)

Categories: 47ms -> 51
Home page: 85 -> 86
Topic: 42 -> 38
Categories Admin: 48 -> 50
Home Admin: 83 -> 84
Topic Admin: 43 -> 39
Boot: 1740ms -> 2484ms
RSS: 358756 -> 640540

This was run with a simple RUBYOPT='--enable-yjit' ruby script/bench.rb -i 100 -s

To get Discourse working with Ruby 3.1 you will need:

diff --git a/Gemfile b/Gemfile
index 9cdfbf21a9..403c428718 100644
--- a/Gemfile
+++ b/Gemfile
@@ -263,3 +263,11 @@ gem 'colored2', require: false
 gem 'maxminddb'
 
 gem 'rails_failover', require: false
+
+if Gem::Version.new(RUBY_VERSION) >= Gem::Version.new("3.0.0")
+  gem 'net-pop', require: false
+  gem 'net-smtp', require: false
+  gem 'net-imap', require: false
+  # waiting on new version of i18n gem
+  gem 'psych', '3.3.2'
+end

It is clear that memory usage and boot time have to take a hit here, but slowing down home page seems concerning. Any ideas on what we should test here and how to narrow this down?

Updated by k0kubun (Takashi Kokubun) 5 months ago

To see what's going on, could you also try running some iterations (at least 10, the default --yjit-call-threashold) before starting performance measurement and/or increasing the number of iterations?

Updated by byroot (Jean Boussier) 5 months ago

Any ideas on what we should test here and how to narrow this down?

You could compile Ruby with YJIT_STATS=1 to have a break down of why and how much YJIT exit back to the interpreter. It's possible that you have some hotspots using constructs that YJIT can't optimize yet.

Updated by noahgibbs (Noah Gibbs) 5 months ago

Possibly related: YJIT released 3.1 was breaking Rails caching: https://bugs.ruby-lang.org/issues/18453

Updated by sam.saffron (Sam Saffron) 5 months ago

@k0kubun (Takashi Kokubun) already doing 20 iterations as a warmup, so it is not likely :cry: https://github.com/discourse/discourse/blob/bbca25e875c36d3541b22c520b6d56ae6322dfbe/script/bench.rb#L192-L192

@byroot (Jean Boussier) @noahgibbs (Noah Gibbs) this is a tough one Ruby trunk is not in a happy state, between the File.exists? removal and cb_data changes and other stuff that was added to trunk getting Discourse to work is not easy. I backported 5414de4b6e4372af832e338f8eb7a9fe8de17c84 by @jhawthorn (John Hawthorn), this is what I got.

Note I had to disable bootsnap cause for some reason I am getting: fiber.so: undefined symbol: ruby_Init_Fiber_as_Coroutine

no yjit -> yjit
categories: 48 -> 48
home: 84 -> 84
topic: 42 -> 39
categories_admin: 48 -> 50
home_admin: 86 -> 82
topic_admin: 42 -> 38
RSS: 293 -> 599

  • Looks improved, however still seeing a regression in categories admin bench so there may be another spot that needs fixing. Additionally perf is flat on a few of the benchmarks which is a concern
  • Oddly bootsnap may be causing an RSS increase 338 -> 370 (on 2.7.5 290 -> 315)
  • Huge memory impact for the fixes in the 3.1.0 tag!
  • 3.2 is going to be a rough upgrade it seems, the File.exists deprecation is going to be very rough, as are a bunch of other changes there.

Updated by byroot (Jean Boussier) 5 months ago

What about YJIT_STATS=1 ? That's the part that will tell us why it's slower.

Oddly bootsnap may be causing an RSS increase 338 -> 370 (on 2.7.5 290 -> 315)

RSS isn't necessarily the best mesure, but yes bootsnap incur increased memory usage for its load path index. A couple years ago I had some ideas to drop the index at the end of the boot process, I'll try look into it when I get back to work next week, fell free to open an issue on Shopify/bootsnap.

fiber.so: undefined symbol: ruby_Init_Fiber_as_Coroutin

From 3.1.0 with just the backport? Sounds weird, something must have gone wrong during compilation.

3.2 is going to be a rough upgrade it seems, the File.exists deprecation is going to be very rough

Meh, we test ruby-head nightly, so I'll likely open PRs about it on popular gems. And for private code it's just a search and replace, no big deal. Also it's just a warning, you can turn it off easily until your dependencies are fixed.

Actions

Also available in: Atom PDF