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

3.x perf #506

Closed
wants to merge 8 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion lib/sprockets/base.rb
Expand Up @@ -100,7 +100,7 @@ def inspect
end

def compress_from_root(uri)
URITar.new(uri, self).compress
URITar.fast_compress(uri, self.root)
end

def expand_from_root(uri)
Expand Down
2 changes: 1 addition & 1 deletion lib/sprockets/cache.rb
Expand Up @@ -60,7 +60,7 @@ def self.default_logger
# cache - A compatible backend cache store instance.
def initialize(cache = nil, logger = self.class.default_logger)
@cache_wrapper = get_cache_wrapper(cache)
@fetch_cache = Cache::MemoryStore.new(1024)
@fetch_cache = Cache::MemoryStore.new
@logger = logger
end

Expand Down
2 changes: 1 addition & 1 deletion lib/sprockets/cache/memory_store.rb
Expand Up @@ -12,7 +12,7 @@ class Cache
#
class MemoryStore
# Internal: Default key limit for store.
DEFAULT_MAX_SIZE = 1000
DEFAULT_MAX_SIZE = 10000

# Public: Initialize the cache store.
#
Expand Down
37 changes: 35 additions & 2 deletions lib/sprockets/digest_utils.rb
Expand Up @@ -84,6 +84,39 @@ def detect_digest_class(bytes)
}
private_constant :ADD_VALUE_TO_DIGEST

def a_v_t_d(c, v, d)
case c
when 'String'.freeze then d << v
when 'FalseClass'.freeze then d << 'FalseClass'.freeze
when 'TrueClass'.freeze then d << 'TrueClass'.freeze
when 'NilClass'.freeze then d << 'NilClass'.freeze
when 'Symbol'.freeze
d << 'Symbol'.freeze
d << v.to_s
when 'Integer'.freeze
d << 'Integer'.freeze
d << v.to_s
when 'Array'.freeze
d << 'Array'.freeze
v.each { |element| a_v_t_d(element.class.name, element, d) }
when 'Hash'.freeze
d << 'Hash'.freeze
v.sort.each { |array| a_v_t_d('Array', array, d) }
when 'Set'.freeze
d << 'Set'.freeze
a_v_t_d('Array',v.to_a, d)
when 'Encoding'.freeze
d << 'Encoding'.freeze
d << v.name
when 'Fixnum'.freeze
d << 'Integer'.freeze
d << v.to_s
when 'Bignum'.freeze
d << 'Integer'.freeze
d << v.to_s
else raise TypeError, "couldn't digest #{c} - #{c.class} -- #{v}"
end
end
# Internal: Generate a hexdigest for a nested JSON serializable object.
#
# This is used for generating cache keys, so its pretty important its
Expand All @@ -94,8 +127,8 @@ def detect_digest_class(bytes)
# Returns a String digest of the object.
def digest(obj)
digest = digest_class.new

ADD_VALUE_TO_DIGEST[obj.class].call(obj, digest)
a_v_t_d(obj.class.name, obj, digest)
# ADD_VALUE_TO_DIGEST[obj.class].call(obj, digest)
digest.digest
end

Expand Down
1 change: 1 addition & 0 deletions lib/sprockets/http_utils.rb
Expand Up @@ -56,6 +56,7 @@ def parse_q_values(values)
#
# Returns Array of matched Strings from available Array or [].
def find_q_matches(q_values, available, &matcher)
return [] if available == []
Copy link
Member

Choose a reason for hiding this comment

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

We're allocating an array everytime this if code is called. We can speed it up by using a pre-allocated constant

require 'benchmark/ips'

x = ""

EMPTY_ARRAY = [].freeze

Benchmark.ips do |x|
  x.report("constant") { x if x == EMPTY_ARRAY }
  x.report("literal ") { x if x == [] }
end

Gives us

Warming up --------------------------------------
            constant   326.041k i/100ms
            literal    306.879k i/100ms
Calculating -------------------------------------
            constant     10.675M (± 7.9%) i/s -     53.145M in   5.011048s
            literal       8.301M (± 7.7%) i/s -     41.429M in   5.022438s

Comparison:
            constant: 10675185.0 i/s
            literal :  8300648.6 i/s - 1.29x  slower

matcher ||= lambda { |a, b| a == b }

matches = []
Expand Down
29 changes: 22 additions & 7 deletions lib/sprockets/uri_tar.rb
Expand Up @@ -13,11 +13,11 @@ def initialize(uri, env)
@root = env.root
@env = env
uri = uri.to_s
if uri.include?("://".freeze)
@scheme, _, @path = uri.partition("://".freeze)
@scheme << "://".freeze
if uri.include?('://'.freeze)
@scheme, _, @path = uri.partition('://'.freeze)
@scheme << '://'.freeze
else
@scheme = "".freeze
@scheme = ''.freeze
@path = uri
end
end
Expand All @@ -35,6 +35,21 @@ def compress
scheme + compressed_path
end

def self.fast_compress(uri, root)
if uri.include?('://'.freeze)
Copy link
Member

Choose a reason for hiding this comment

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

We don't need all the freeze calls. We're using the magic comment in the files:

# frozen_string_literal: true

Copy link
Author

Choose a reason for hiding this comment

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

oh, right, missed that

scheme, _, path = uri.partition('://'.freeze)
scheme << '://'.freeze
else
scheme = ''.freeze
path = uri
end
if compressed_path = PathUtils.split_subpath(root, path)
scheme + compressed_path
else
scheme + path
end
end

# Internal: Tells us if we are using an absolute path
#
# Nix* systems start with a `/` like /Users/schneems.
Expand Down Expand Up @@ -66,7 +81,7 @@ def expand
else
# We always want to return an absolute uri,
# make sure the path starts with a slash.
scheme + File.join("/".freeze, root, path)
scheme + File.join('/'.freeze, root, path)
end
end
end
Expand All @@ -82,8 +97,8 @@ def expand
# Returns String
def compressed_path
# windows
if !@root.start_with?("/".freeze) && path.start_with?("/".freeze)
consistent_root = "/".freeze + @root
if !@root.start_with?('/'.freeze) && path.start_with?('/'.freeze)
consistent_root = '/'.freeze + @root
else
consistent_root = @root
end
Expand Down
8 changes: 4 additions & 4 deletions lib/sprockets/uri_utils.rb
Expand Up @@ -48,7 +48,7 @@ def split_file_uri(uri)
path.force_encoding(Encoding::UTF_8)

# Hack for parsing Windows "file:///C:/Users/IEUser" paths
path.gsub!(/^\/([a-zA-Z]:)/, '\1'.freeze)
path = path[1..-1] if File::ALT_SEPARATOR

[scheme, host, path, query]
end
Expand All @@ -59,7 +59,7 @@ def split_file_uri(uri)
def join_file_uri(scheme, host, path, query)
str = "#{scheme}://"
str << host if host
path = "/#{path}" unless path.start_with?("/")
path = "/#{path}" unless path.start_with?('/'.freeze)
str << URI::Generic::DEFAULT_PARSER.escape(path)
str << "?#{query}" if query
str
Expand All @@ -72,7 +72,7 @@ def join_file_uri(scheme, host, path, query)
# Returns true or false.
def valid_asset_uri?(str)
# Quick prefix check before attempting a full parse
str.start_with?("file://") && parse_asset_uri(str) ? true : false
str.start_with?('file://'.freeze) && parse_asset_uri(str) ? true : false
rescue URI::InvalidURIError
false
end
Expand Down Expand Up @@ -109,7 +109,7 @@ def parse_asset_uri(uri)
#
# Returns String URI.
def build_asset_uri(path, params = {})
join_file_uri("file", nil, path, encode_uri_query_params(params))
join_file_uri('file', nil, path, encode_uri_query_params(params))
end

# Internal: Parse file-digest dependency URI.
Expand Down
27 changes: 16 additions & 11 deletions test/test_uri_utils.rb
Expand Up @@ -45,15 +45,16 @@ def test_split_file_uri

parts = split_file_uri("file:///usr/local/var/github/app/assets/javascripts/application.js")
assert_equal ['file', nil, '/usr/local/var/github/app/assets/javascripts/application.js', nil], parts
if DOSISH
parts = split_file_uri("file:///C:/Documents%20and%20Settings/davris/FileSchemeURIs.doc")
assert_equal ['file', nil, 'C:/Documents and Settings/davris/FileSchemeURIs.doc', nil], parts

parts = split_file_uri("file:///C:/Documents%20and%20Settings/davris/FileSchemeURIs.doc")
assert_equal ['file', nil, 'C:/Documents and Settings/davris/FileSchemeURIs.doc', nil], parts
parts = split_file_uri("file:///D:/Program%20Files/Viewer/startup.htm")
assert_equal ['file', nil, 'D:/Program Files/Viewer/startup.htm', nil], parts

parts = split_file_uri("file:///D:/Program%20Files/Viewer/startup.htm")
assert_equal ['file', nil, 'D:/Program Files/Viewer/startup.htm', nil], parts

parts = split_file_uri("file:///C:/Program%20Files/Music/Web%20Sys/main.html?REQUEST=RADIO")
assert_equal ['file', nil, 'C:/Program Files/Music/Web Sys/main.html', 'REQUEST=RADIO'], parts
parts = split_file_uri("file:///C:/Program%20Files/Music/Web%20Sys/main.html?REQUEST=RADIO")
assert_equal ['file', nil, 'C:/Program Files/Music/Web Sys/main.html', 'REQUEST=RADIO'], parts
end
end

def test_join_uri_path
Expand Down Expand Up @@ -98,8 +99,10 @@ def test_parse_file_paths
parse_asset_uri("file:///usr/local/var/github/app/assets/javascripts/application.js")
assert_equal ["/usr/local/var/github/app/assets/javascripts/foo bar.js", {}],
parse_asset_uri("file:///usr/local/var/github/app/assets/javascripts/foo%20bar.js")
assert_equal ["C:/Users/IEUser/Documents/github/app/assets/javascripts/application.js", {}],
parse_asset_uri("file:///C:/Users/IEUser/Documents/github/app/assets/javascripts/application.js")
if DOSISH
assert_equal ["C:/Users/IEUser/Documents/github/app/assets/javascripts/application.js", {}],
parse_asset_uri("file:///C:/Users/IEUser/Documents/github/app/assets/javascripts/application.js")
end
end

def test_parse_query_params
Expand Down Expand Up @@ -152,8 +155,10 @@ def test_parse_file_digest_uri
parse_file_digest_uri("file-digest:///usr/local/var/github/app/assets/javascripts/application.js")
assert_equal "/usr/local/var/github/app/assets/javascripts/foo bar.js",
parse_file_digest_uri("file-digest:///usr/local/var/github/app/assets/javascripts/foo%20bar.js")
assert_equal "C:/Users/IEUser/Documents/github/app/assets/javascripts/application.js",
parse_file_digest_uri("file-digest:///C:/Users/IEUser/Documents/github/app/assets/javascripts/application.js")
if DOSISH
assert_equal "C:/Users/IEUser/Documents/github/app/assets/javascripts/application.js",
parse_file_digest_uri("file-digest:///C:/Users/IEUser/Documents/github/app/assets/javascripts/application.js")
end
end

def test_build_file_digest_uri
Expand Down