Skip to content

Conversation

jan-bw
Copy link

@jan-bw jan-bw commented Oct 22, 2024

Description

Use the Rainbow gem directly (i.e. Rainbow("text").red) instead of relying on the String monkey patching.

I checked for usage of the following methods:

  • background
  • bg
  • black
  • blink
  • blue
  • bold
  • bright
  • color
  • cross_out
  • cyan
  • dark
  • faint
  • fg
  • foreground
  • green
  • hide
  • inverse
  • italic
  • magenta
  • red
  • reset
  • strike
  • underline
  • white
  • yellow

Those are the basic methods defined by the rainbow. I skipped the methods covered by the problematic "method_missing".

Motivation and Context

Fix #130

As raised in the #130, the release 1.4.0 impacted performance of a couple apps. The underlying issue is that the rainbow gem is refining the String class and that the method_missing is overwritten.

The refine seems have some performance issues on some of the ruby releases: https://bugs.ruby-lang.org/issues/18572

This PR is a proposal for mitigating this issue without a need of fixing rainbow gem nor upgrading the ruby version. If the changes here aren't too annoying maybe that could be a way to go? :)

How Has This Been Tested?

I run it against one of the endpoint of the affected application.

average response of 100 requests:

  • v1.3.0: 816.07ms
  • v1.4.0: 951.84ms
  • patch: 816.83ms

I am not sure whether there is a way of testing this in any other way 🤷

Copy link
Member

@etagwerker etagwerker left a comment

Choose a reason for hiding this comment

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

@jan-bw Looks good, thank you!

@etagwerker etagwerker merged commit 1e27f04 into fastruby:main Oct 23, 2024
6 checks passed
@JuanVqz
Copy link
Member

JuanVqz commented Oct 23, 2024

@jan-bw we really appreciate your contribution, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG] Upgrading to 1.4.0 increases our request time from ~200ms to 3-6s on every page load in development

3 participants