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.

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

fix(performance): comment all log_debug #56

Merged
merged 1 commit into from
Apr 14, 2023

Conversation

abrisse
Copy link
Member

@abrisse abrisse commented Apr 14, 2023

Hello @gkellogg,

While investigating some memory issues I discovered that the log_debug calls generate a lot of objects. Half of them were already commented so this PR comments all the others.

I did a small benchmark with memory_profiler:

allocated memory by gem (current)
-------------------------------------------------------
  48.30 MB  json-ld
  41.03 MB  rdf-ac80b2f2b82e

allocated memory by gem (with all log_debug commented)
-------------------------------------------------------
  43.33 MB  json-ld (- 10%)
  15.79 MB  rdf-ac80b2f2b82e (- 42%)

The 3 lines that stores a lot of objects:

  • 9.12 MB rdf/util/logger.rb:181
  • 7.97 MB rdf/util/logger.rb:224
  • 6.98 MB rdf/util/logger.rb:180

This is partly because the RDF utils functions logger_debug / logger_common use splat operators which is quite costly, so I think the PR provide the best ROI.

@coveralls
Copy link

Coverage Status

Coverage: 84.444% (-1.0%) from 85.432% when pulling 18f84e8 on PerfectMemory:fix/debug-log into 742eecb on ruby-rdf:develop.

@gkellogg
Copy link
Member

Interesting and unfortunate. I've tried to decrease the impact of log_* calls by leaving costly evaluation in the block, but there's no good way to avoid splat arguments. I've gone back and forth about commenting out logging, and it can be useful when trying to debug gnarly corner-cases. But, the log_debug calls probably make up the bulk and shouldn't be as necessary to get basic algorithm tracing as log_info, for example.

@gkellogg gkellogg merged commit 8d8cd36 into ruby-rdf:develop Apr 14, 2023
9 checks passed
@abrisse abrisse deleted the fix/debug-log branch April 17, 2023 07:31
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.

None yet

3 participants