Skip to content

[equality] Comment out debugging statements in lib.quality/=#65641

Merged
alexander-yakushev merged 1 commit into
masterfrom
equality-debug
Apr 30, 2026
Merged

[equality] Comment out debugging statements in lib.quality/=#65641
alexander-yakushev merged 1 commit into
masterfrom
equality-debug

Conversation

@alexander-yakushev
Copy link
Copy Markdown
Member

Having these logging statements has significant impact on allocation rate, even if the requested logging level is disabled. That's because the logging system has to construct the logger object anyway.

I'd argue that having those logging statements always on isn't helpful because enabling debug log in that function would create immense noise because of how often this equality function is being called.

@alexander-yakushev alexander-yakushev requested review from a team and bshepherdson November 10, 2025 00:29
@alexander-yakushev alexander-yakushev self-assigned this Nov 10, 2025
@alexander-yakushev alexander-yakushev added the backport Automatically create PR on current release branch on merge label Nov 10, 2025
@edpaget
Copy link
Copy Markdown
Contributor

edpaget commented Nov 10, 2025

Could we extend the log functions to avoid constructing the logger if we're not at the requested debug level?

@alexander-yakushev
Copy link
Copy Markdown
Member Author

@edpaget The problem is that knowing if we are at the requested debug level for this particular process/namespace/file requires constructing a logger (or at least reading into the logging config). Maybe, it is somehow possible to make this check leaner, but it will still be an overhead which is quite redundant for a such heavily called function.

@github-actions github-actions Bot added the Stale Stale PRs that will be auto-closed label Feb 12, 2026
@github-actions github-actions Bot closed this Feb 27, 2026
@alexander-yakushev alexander-yakushev requested a review from a team as a code owner April 12, 2026 19:19
@metabase metabase deleted a comment from github-actions Bot Apr 12, 2026
@metabase metabase deleted a comment from github-actions Bot Apr 12, 2026
@github-actions github-actions Bot removed the Stale Stale PRs that will be auto-closed label Apr 13, 2026
Copy link
Copy Markdown
Member

@camsaul camsaul left a comment

Choose a reason for hiding this comment

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

Can you please add a note why you're commenting these out (for performance reasons)? Other than that it looks good to me.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Apr 29, 2026

e2e tests failed on 5f41f94799762bb172e41555d04ea49a7c59dd1c-1

e2e test run

File Test Name
source-replacement.cy.spec.ts scenarios > data-studio > source replacement > Native queries > replaces a table referenced in a native SQL question
embed-parameters.cy.spec.ts scenarios > embedding > sdk iframe embed setup > embed parameters > resources without parameters > shows no parameters message for dashboards without parameters
maps.cy.spec.js scenarios > visualizations > maps > should preserve zoom and pan after resize (#11211)
query-external.cy.spec.js scenarios > question > query > external > can query Mongo database

@alexander-yakushev alexander-yakushev merged commit 1281f41 into master Apr 30, 2026
752 of 764 checks passed
@alexander-yakushev alexander-yakushev deleted the equality-debug branch April 30, 2026 22:42
github-automation-metabase added a commit that referenced this pull request Apr 30, 2026
…ity/=" (#73492)

[equality] Comment out debugging statements in lib.quality/= (#65641)

Co-authored-by: Oleksandr Yakushev <alex@bytopia.org>
@github-actions github-actions Bot added this to the 0.61 milestone Apr 30, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport Automatically create PR on current release branch on merge

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants