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

Avoid unnecessary string allocation when calculating length #1454

Merged
merged 1 commit into from Sep 3, 2022
Merged

Avoid unnecessary string allocation when calculating length #1454

merged 1 commit into from Sep 3, 2022

Conversation

ghost
Copy link

@ghost ghost commented Aug 18, 2022

Description

Hi, I am trying to optimize YARD a bit to speed up documentation runs on a large codebase.

The codebase is private, so I am using https://github.com/watir/watir/ as a smaller stand-in to report benchmarking numbers from https://github.com/SamSaffron/memory_profiler.

Test set-up

  • Ruby 2.7.6 (installed via rvm on Linux)
  • Watir version: 293bed2b57c
  • MemoryProfiler version: 1.0.0
  • YARD baseline: 0a55093
  • Test command:
    • ruby-memory-profiler --no-color --retained-strings=500 --allocated-strings=500 --max=300 -o prof.log ./run-yard.rb stats
      • run-yard.rb is a smaller version of /bin/yard (MemoryProfiler was not able to run /bin/yard)
      • I am using the stats command to avoid memory exhaustion

Performance data

  • I don't have concrete performance data because the stats command doesn't seem to trigger this code path
  • On the private codebase, the loop body was executed ~375k times, so I think this optimization is worthwhile

Completed Tasks

  • I have read the Contributing Guide.
  • The pull request is complete (implemented / written).
  • Git commits have been cleaned up (squash WIP / revert commits).
  • I wrote tests and ran bundle exec rake locally (if code is attached to PR).

@lsegal lsegal merged commit 134be75 into lsegal:main Sep 3, 2022
@lsegal
Copy link
Owner

lsegal commented Sep 3, 2022

Thank you for this PR!

@ghost ghost deleted the perf/dont-generate-unneeded-string branch September 12, 2022 16:36
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

1 participant