Skip to content

Limit object allocations #115

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.

Sign up for GitHub

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

Merged
pythonicrubyist merged 2 commits into from
May 3, 2023
Merged

Limit object allocations #115

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
5d87d7a
add missing frozen_string_litteral
ThomasSevestre May 3, 2023
80c7311
limit string allocations in Creek::Sheet#rows_generator
ThomasSevestre May 3, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions lib/creek/book.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
# frozen_string_literal: true

require 'zip/filesystem'
require 'nokogiri'
require 'date'
Expand Down
2 changes: 2 additions & 0 deletions lib/creek/drawing.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
# frozen_string_literal: true

require 'pathname'

module Creek
Expand Down
2 changes: 2 additions & 0 deletions lib/creek/shared_strings.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
# frozen_string_literal: true

require 'zip/filesystem'
require 'nokogiri'

Expand Down
16 changes: 12 additions & 4 deletions lib/creek/sheet.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,10 @@ def rows_generator include_meta_data=false, use_simple_rows_format=false
cell_style_idx = nil
@book.files.file.open(path) do |xml|
prefix = ''
name_row = "row"
name_c = "c"
name_v = "v"
name_t = "t"
Nokogiri::XML::Reader.from_io(xml).each do |node|
if prefix.empty? && node.namespaces.any?
namespace = node.namespaces.detect{|_key, uri| uri == SPREADSHEETML_URI }
Comment on lines 107 to 108
Copy link

@md5 md5 May 3, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the case that prefix has not been set and there are namespaces that are not SPREADSHEETML_URI, the code is still going to repeatedly set the name_* variables.

Perhaps this would make sense?

Suggested change
if prefix.empty? && node.namespaces.any?
namespace = node.namespaces.detect{|_key, uri| uri == SPREADSHEETML_URI }
if prefix.empty? && (namespace = node.namespaces.detect{|_key, uri| uri == SPREADSHEETML_URI }).present?

Copy link

@md5 md5 May 3, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess that suggestion leaves some quirks around the if namespace && ... check below...

Copy link
Contributor Author

@ThomasSevestre ThomasSevestre May 3, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the case that prefix has not been set and there are namespaces that are not SPREADSHEETML_URI, the code is still going to repeatedly set the name_* variables.

Perhaps this would make sense?

I don't know if this is a common usecase.

If it is, we need to track prefix changes in order to handle new prefix and prefix reset :

if prefix != old_prefix
  name_row = "#{prefix}row"
  name_c = "#{prefix}c"
  name_v = "#{prefix}v"
  name_t = "#{prefix}t"
end

Expand All @@ -107,13 +111,17 @@ def rows_generator include_meta_data=false, use_simple_rows_format=false
else
''
end
name_row = "#{prefix}row"
name_c = "#{prefix}c"
name_v = "#{prefix}v"
name_t = "#{prefix}t"
end
if node.name == "#{prefix}row" && node.node_type == opener
if node.name == name_row && node.node_type == opener
row = node.attributes
row['cells'] = {}
cells = {}
y << (include_meta_data ? row : cells) if node.self_closing?
elsif node.name == "#{prefix}row" && node.node_type == closer
elsif node.name == name_row && node.node_type == closer
processed_cells = fill_in_empty_cells(cells, row['r'], cell, use_simple_rows_format)
@headers = processed_cells if with_headers && row['r'] == HEADERS_ROW_NUMBER

Expand All @@ -127,11 +135,11 @@ def rows_generator include_meta_data=false, use_simple_rows_format=false

row['cells'] = processed_cells
y << (include_meta_data ? row : processed_cells)
elsif node.name == "#{prefix}c" && node.node_type == opener
elsif node.name == name_c && node.node_type == opener
cell_type = node.attributes['t']
cell_style_idx = node.attributes['s']
cell = node.attributes['r']
elsif ["#{prefix}v", "#{prefix}t"].include?(node.name) && node.node_type == opener
elsif (node.name == name_v || node.name == name_t) && node.node_type == opener
unless cell.nil?
node.read
cells[cell] = convert(node.value, cell_type, cell_style_idx)
Expand Down
2 changes: 2 additions & 0 deletions lib/creek/styles.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
# frozen_string_literal: true

module Creek
class Styles
attr_accessor :book
Expand Down
2 changes: 2 additions & 0 deletions lib/creek/styles/converter.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
# frozen_string_literal: true

require 'set'

module Creek
Expand Down
2 changes: 2 additions & 0 deletions lib/creek/styles/style_types.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
# frozen_string_literal: true

# https://github.com/hmcgowan/roo/blob/master/lib/roo/excelx.rb
# https://github.com/woahdae/simple_xlsx_reader/blob/master/lib/simple_xlsx_reader.rb#L231
module Creek
Expand Down
2 changes: 2 additions & 0 deletions lib/creek/utils.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
# frozen_string_literal: true

module Creek
module Utils
def expand_to_rels_path(filepath)
Expand Down
2 changes: 2 additions & 0 deletions lib/creek/version.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
# frozen_string_literal: true

module Creek
VERSION = "2.6.2"
end