Skip to content
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

Jump to bottom

sql_color is slow for long sql queries #38685

Closed
spjsschl opened this issue Mar 9, 2020 · 2 comments · Fixed by #38690
Closed

sql_color is slow for long sql queries #38685

spjsschl opened this issue Mar 9, 2020 · 2 comments · Fixed by #38690

Comments

@spjsschl
Copy link

spjsschl commented Mar 9, 2020

This has been brought up here before #24330 but is still an issue occasionally. The application of these regular expressions can sometimes dominate the request time, including the SQL server processing the query and instantiating a lot of AR-objects as a result. I have had cases in my app where disabling sql_color has reduced request times for a request from 4.8 to 1.2 seconds.

Steps to reproduce

  • Generate a long sql query and send it through AR (e.g. find_by_sql)
  • Observe that a lot of time is spent in ruby case matching using Regexp.=== (for example using some profiler)
  • Narrow this down to inefficient regular expressions being applied by sql_color

Expected behavior

sql_color should not slow down query processing (in production)

Actual behavior

I am unsure.
Maybe sql_color can be configured (perhaps optionally) to be disabled in production, maybe the regular expressions can be made faster?

System configuration

Rails version: 6.0.2.1

Ruby version: 2.6.3
@rafaelfranca
Copy link
Member

We should include an option to disable that. Can you work in a PR?

@abhaynikam
Copy link
Contributor

@rafaelfranca @spjsschl Can I open a PR for this? I did the patch for it and used colorize_logging config to disable the unnecessary sql_color call happening when colorize_logging is disabled.

@abhaynikam abhaynikam mentioned this issue Mar 10, 2020
Merged
abhaynikam added a commit to abhaynikam/rails that referenced this issue Mar 11, 2020
@abhaynikam
Disable colorize logging of SQL and reduce unnecessary invokation of …
85b5cd3 
…sql_color matching

When the `colorize_logging` is disabled,
logs do not colorize the SQL queries.
But the `sql_color` method is always
invoked which due to regex matching results
in slow queries.

This PR fixes rails#38685 and removes
unnecessary invokation of `sql_color`
method when `colorize_logging` is disabled
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
3 participants
@rafaelfranca @spjsschl @abhaynikam