Reduce memory usage during gem statup #1090
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
marcotc
added
core
marcotc
commented
Jun 18, 2020
| @@ -26,7 +26,8 @@ def option(name, meta = {}, &block) | |||
| builder = OptionDefinition::Builder.new(name, meta, &block) | |||
| options[name] = builder.to_definition.tap do | |||
| # Resolve and define helper functions | |||
| helpers = default_helpers(name).merge(builder.helpers) | |||
| helpers = default_helpers(name) | |||
| helpers = helpers.merge(builder.helpers) unless builder.helpers.empty? | |||
There was a problem hiding this comment.
Merging with an empty hash creates an unnecessary copy of the original hash.
| o.default false | ||
| o.on_set do |enabled| | ||
| # Enable rich debug print statements | ||
| require 'pp' if enabled |
There was a problem hiding this comment.
Loading of pp is now lazy, as it is only used in production ddtrace when debug diagnostics are enabled.
Although pp is commonly used in testing and development, the host application is not guaranteed to need it in production mode.
ericmustin
approved these changes
Sep 17, 2020
|
@marcotc None of these test failures seem related afaik |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR introduces a few changes guide by the results from our new startup benchmarks: #1062.
The largest memory footprint we have as a gem is loading external dependencies (through
require) and loading our own Ruby files.Most of our dependencies will likely be used by the host application (e.g.
thread,logger,sockets), so their overhead is shared with the host.A few "unique" dependencies showed up, like
msgpack, but they are required for the correct function of the tracer.Another PR will follow with changes to load integration code dynamically, which should have the largest impact, based on my local testing.
This PR is an aggregation of a few changes that showed up as relevant enough in the benchmarks and that don't require large rework, nor introduce large risks.
This PR should have the following performance impact: