-
Notifications
You must be signed in to change notification settings - Fork 75
fix odd convert from string to integer and float #175
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 odd convert from string to integer and float #175
Conversation
5ea5076 to
f60229c
Compare
benchmark Integer(Float(value)) VS Integer(BigDecimal(value))require 'benchmark/ips'
require 'bigdecimal'
test_cases = {
'small_integer' => '123',
'large_integer' => '9007199254740993',
'decimal' => '123.456',
'large_decimal' => '1234567.89012345',
'scientific' => '1.23e2',
'scientific_large' => '1.23e10'
}
test_cases.each do |name, value|
clean_value = value.delete(',')
puts "Testing: #{name} (#{value})"
puts "-" * 80
Benchmark.ips do |x|
x.config(time: 5, warmup: 2)
x.report("Integer(Float(value))") do
Integer(Float(clean_value))
end
x.report("Integer(BigDecimal(value))") do
Integer(BigDecimal(clean_value))
end
x.compare!
end
puts
end
|
|
created an issue upstream for the difference of jruby and cruby jruby/jruby#9044 |
lib/logstash/filters/mutate.rb
Outdated
| # floating point number. BigDecimal() can't parse hex string | ||
| return Integer(BigDecimal(value)) if value.count('.') == 1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
At this point we're only left with decimals and exponentials, and I think that to_i can handle anything that is not exponential, and Float handle the exponentials?
| # floating point number. BigDecimal() can't parse hex string | |
| return Integer(BigDecimal(value)) if value.count('.') == 1 | |
| return Float(value).to_i if value.include?("e") | |
| return value.to_i |
This change makes the performance hit less significant:
Testing: small_integer (123)
Comparison:
Test Original: 2117612.5 i/s
Test Change: 1971480.7 i/s - 1.07x slower
Testing: large_integer (9007199254740993)
Comparison:
Test Original: 1780093.5 i/s
Test Change: 1616786.2 i/s - 1.10x slower
Testing: decimal (123.456)
Comparison:
Test Original: 2104084.0 i/s
Test Change: 1956469.1 i/s - 1.08x slower
Testing: large_decimal (1234567.89012345)
Comparison:
Test Original: 2085646.4 i/s
Test Change: 1905504.3 i/s - 1.09x slower
Testing: scientific (1.23e2)
Comparison:
Test Original: 2004532.4 i/s
Test Change: 1687208.7 i/s - 1.19x slower
Testing: scientific_large (1.23e10)
Comparison:
Test Original: 2115260.8 i/s
Test Change: 1680841.1 i/s - 1.26x slower
jsvd
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Fix
convertto covert hexadecimal notation and scientific notation string to float and integerAdd support for hexadecimal float notation (0x1.edcdp6) and scientific
notation (1.230000e+02) in convert option for
integerandfloat. Includes manualsign parsing for JRuby compatibility and BigDecimal for precision.
Fix: #174
JRuby cannot parse negative hex string to float
Float("-0x7b")and uppsercase hexFloat("0X1A"), but CRuby can.JRuby 9.4.5.0 Logstash 8.13 also cannot parse floating point hex string
Float("0x1.edcdp6")This commit adds parsing for signed hex strings for Logstash version >= 8.14. For older versions, type conversions fall back to the old implementation.