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

Add frozen_string_literal comment #13

Merged
merged 1 commit into from
Nov 30, 2022

Conversation

elebow
Copy link
Contributor

@elebow elebow commented Nov 29, 2022

This change adds frozen_string_literal: true. Frozen strings are allocated once at parse-time and reused. This saves a substantial number of memory allocations and String instantiations. This magic comment is safe to use in all versions of Ruby.

Benchmarking using the same snippet as in #12 shows a 1.2% speedup in line generation. Note that these benchmarks were performed with #12 applied.

require "benchmark"

Benchmark.bmbm do |bm_out|
	bm_out.report { 5_000_000.times { CSVSafe.generate_line(["|yes", "no"]) } }
end

Legacy:

       user     system      total        real
  89.457235   0.255369  89.712604 ( 89.747471)

This branch:

       user     system      total        real
  88.287842   0.264821  88.552663 ( 88.604517)

The #starts_with_special_character? method itself is about 37% faster, using a similar benchmark.

on-behalf-of: @Cofense <oss@cofense.com>
@elebow
Copy link
Contributor Author

elebow commented Nov 29, 2022

Some smarter benchmarks, using the library more effectively:

csvsafe = CSVSafe.new($stderr, encoding: Encoding.find("utf-8"))

Benchmark.bmbm do |bm_out|
  bm_out.report { 5_000_000.times { csvsafe << ["|yes", "no"] } }
end

baseline:

       user     system      total        real
  22.137823   3.364592  25.502415 ( 25.550722)

start_with? (6.3% faster than baseline):

       user     system      total        real
  20.558799   3.466152  24.024951 ( 24.083241)

start_with? and frozen_string_literal (14.5% faster than baseline):

       user     system      total        real
  18.554075   3.299231  21.853306 ( 21.910121)

@zvory
Copy link
Owner

zvory commented Nov 30, 2022

thank you

@zvory zvory merged commit 3432470 into zvory:master Nov 30, 2022
@zvory
Copy link
Owner

zvory commented Nov 30, 2022

rolled out in #14 and released to rubygems.

@zvory
Copy link
Owner

zvory commented Nov 30, 2022

@elebow just curious, where do you use csv-safe?

@elebow elebow deleted the frozen-string-literal branch November 30, 2022 04:07
@elebow
Copy link
Contributor Author

elebow commented Jan 3, 2023

We use csv-safe in several products at Cofense to ensure generated CSVs behave predictably in Excel.

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.

2 participants