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

Improve our SafeDup module #2960

Merged
merged 9 commits into from
Jul 14, 2023
24 changes: 20 additions & 4 deletions lib/datadog/core/backport.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,12 @@ module Datadog
module Core
# This module is used to provide features from Ruby 2.5+ to older Rubies
module BackportFrom25
module_function

if ::String.method_defined?(:delete_prefix)
def string_delete_prefix(string, prefix)
def self.string_delete_prefix(string, prefix)
string.delete_prefix(prefix)
end
else
def string_delete_prefix(string, prefix)
def self.string_delete_prefix(string, prefix)
prefix = prefix.to_s
if string.start_with?(prefix)
string[prefix.length..-1] || raise('rbs-guard: String#[] is non-nil as `prefix` is guaranteed present')
Expand All @@ -21,5 +19,23 @@ def string_delete_prefix(string, prefix)
end
end
end

# This module is used to provide features from Ruby 2.4+ to older Rubies
module BackportFrom24
if RUBY_VERSION < '2.4'
def self.dup(value)
case value
when NilClass, TrueClass, FalseClass, Numeric
value
else
value.dup
end
end
else
def self.dup(value)
value.dup
end
end
end
end
end
47 changes: 27 additions & 20 deletions lib/datadog/core/utils/safe_dup.rb
Original file line number Diff line number Diff line change
@@ -1,40 +1,47 @@
require_relative '../backport'

module Datadog
module Core
module Utils
# Helper methods for safer dup
module SafeDup
if RUBY_VERSION < '2.2' # nil.dup only fails in Ruby 2.1
# Ensures #initialize can call nil.dup safely
module RefineNil
refine NilClass do
def dup
self
end
end
end

using RefineNil
end

# String#+@ was introduced in Ruby 2.3
if String.method_defined?(:+@) && String.method_defined?(:-@)
def self.frozen_or_dup(v)
# If the string is not frozen, the +(-v) will:
# - first create a frozen deduplicated copy with -v
# - then it will dup it more efficiently with +v
v.frozen? ? v : +(-v)
# For the case of a String we use the methods +@ and -@.
# Those methods are only for String objects
# they are faster and chepaer on the memory side.
# Check the benchmark on
# https://github.com/DataDog/dd-trace-rb/pull/2704
if v.is_a?(String)
# If the string is not frozen, the +(-v) will:
# - first create a frozen deduplicated copy with -v
# - then it will dup it more efficiently with +v
v.frozen? ? v : +(-v)
else
v.frozen? ? v : Core::BackportFrom24.dup(v)
end
end

def self.frozen_dup(v)
-v if v
# For the case of a String we use the methods -@
# That method are only for String objects
# they are faster and chepaer on the memory side.
# Check the benchmark on
# https://github.com/DataDog/dd-trace-rb/pull/2704
if v.is_a?(String)
-v if v
else
v.frozen? ? v : Core::BackportFrom24.dup(v).freeze
end
end
else
def self.frozen_or_dup(v)
v.frozen? ? v : v.dup
v.frozen? ? v : Core::BackportFrom24.dup(v)
end

def self.frozen_dup(v)
v.frozen? ? v : v.dup.freeze
v.frozen? ? v : Core::BackportFrom24.dup(v).freeze
end
end
end
Expand Down
18 changes: 3 additions & 15 deletions lib/datadog/tracing/span_operation.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
require_relative '../core/environment/identity'
require_relative '../core/utils'
require_relative '../core/utils/time'
require_relative '../core/utils/safe_dup'

require_relative 'event'
require_relative 'metadata'
Expand Down Expand Up @@ -434,19 +435,6 @@ def message
:parent,
:span

if RUBY_VERSION < '2.2' # nil.dup only fails in Ruby 2.1
# Ensures #initialize can call nil.dup safely
module RefineNil
refine NilClass do
def dup
self
end
end
end

using RefineNil
end

# Create a Span from the operation which represents
# the finalized measurement. We #dup here to prevent
# mutation by reference; when this span is returned,
Expand All @@ -457,8 +445,8 @@ def build_span
duration: duration,
end_time: @end_time,
id: @id,
meta: meta.dup,
metrics: metrics.dup,
meta: Core::Utils::SafeDup.frozen_or_dup(meta),
metrics: Core::Utils::SafeDup.frozen_or_dup(metrics),
ivoanjo marked this conversation as resolved.
Show resolved Hide resolved
parent_id: @parent_id,
resource: @resource,
service: @service,
Expand Down
6 changes: 5 additions & 1 deletion sig/datadog/core/backport.rbs
Original file line number Diff line number Diff line change
@@ -1,7 +1,11 @@
module Datadog
module Core
module BackportFrom25
def self?.string_delete_prefix: (::String string, ::String prefix) -> ::String
def self.string_delete_prefix: (::String string, ::String prefix) -> ::String
end

module BackportFrom24
def self.dup: (Object value) -> Object
end
end
end
Loading