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

Performance sucker! -> Get performance doubled! #503

Closed
janbiedermann opened this issue Oct 12, 2017 · 15 comments
Closed

Performance sucker! -> Get performance doubled! #503

janbiedermann opened this issue Oct 12, 2017 · 15 comments

Comments

@janbiedermann
Copy link

janbiedermann commented Oct 12, 2017

sprockets-3.7.1/lib/sprockets/uri_utils.rb
in
def split_file_uri(uri)

Line 51

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

is a performance sucker. Replacing it with this:

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

DOUBLES performance for me, that is injavascript_include_tag 'applications.js', generating the <script> tag, time going down from 3 seconds to 1.5 seconds, in rails, development environment, with opal-rails and ruby-hyperloop.org.

@janbiedermann janbiedermann changed the title Performance sucker! Performance sucker! -> Get performance doubled! Oct 12, 2017
@schneems
Copy link
Member

Send me a PR?

@janbiedermann
Copy link
Author

Ok, a little later, currently busy investigating another, maybe related issue

@schneems
Copy link
Member

Also strings can start with file:/// or file://

@janbiedermann
Copy link
Author

janbiedermann commented Oct 12, 2017

yes, i am looking for file:///c:, the second :, because its about windows, and i don't use windows,
and the regexp does so too, its looking for /c:, if i understand it correctly, if it runs on windows, in [9] will be a :, if it doesn't, no need to gsub

@janbiedermann
Copy link
Author

janbiedermann commented Oct 12, 2017

Note to self: Try with this:

require 'rbconfig'
@platform = case RbConfig::CONFIG['host_os']
           when /darwin/
              OSX
           when /win/
              WINDOWS
end`

thanks to @fkchang

@schneems
Copy link
Member

I'm saying that while doing the check we have to check for a colon in both index path[9] and path[8] since the start of the string could have 2 slashes or 3.

Also we can use a straight equal == instead of include?(':') as it will do less work and be faster.

@schneems
Copy link
Member

We generally use File::ALT_SEPARATOR to detect windows

if File::ALT_SEPARATOR

@janbiedermann
Copy link
Author

Oh, yes, before i had

if path[7..9].include?(':')

thats where the include came from, then i thought [7..8] is not legal and changed it to [9]

@janbiedermann
Copy link
Author

janbiedermann commented Oct 12, 2017

Great, then File::ALT_SEPARATOR it shall be

@ahorek
Copy link
Contributor

ahorek commented Oct 12, 2017

hi, the comment is misleading. 'path' variable contains already parsed uri, so it actualy looks like this

# Hack for parsing Windows "/C:/Users/IEUser" paths
if File::ALT_SEPARATOR
  path = path[1..-1]
end

@janbiedermann
Copy link
Author

janbiedermann commented Oct 14, 2017

patches and numbers
#506
sorry, forgot about path[1..-1]

This was referenced Oct 14, 2017
@janbiedermann
Copy link
Author

janbiedermann commented Oct 18, 2017

Here are numbers, effects per patch vs. stock 3.7.1 for a large set of assets like in opal-rails/ruby-hyperloop on my machine, large rails app, time taken for a javascript_include_tag, development environment, ruby-2.4.2, macOS, assets.debug = true, FileStore, best out of 10:

without compile

3.7.1 stock:         5.08s
3.7.1 3.x perf #506: 2.51s, ~2x
3.7.1 cache:         3.67s, ~1.38x
3.7.1 regexp:        3.85s, ~1.31x
conclusion

regexp patch and cache size patch are most effective, performance doubled
digest, fqmatch, fastcomp each give mixed results, non conclusive

opal recompiling an *.rb

3.7.1 stock:          7.08s
3.7.1 3.x perf #506:  3.09s, ~2.2x
3.7.1 cache:          5.38s, ~1.31x
3.7.1 regexp:         5.07s, ~1.38x
conclusion

regexp patch and cache size patch are most effective, performance doubled
digest, fqmatch, fastcomp each give mixed results, non conclusive

@janbiedermann
Copy link
Author

janbiedermann commented Oct 18, 2017

without compile

3.7.1 3.x perf proper #509: 2.66s, ~1.9x

opal recompiling an *.rb

3.7.1 3.x perf proper #509: 3.17s, ~2.2%

@janbiedermann
Copy link
Author

*_proper containing only the cache adjustment and regexp patch

@janbiedermann
Copy link
Author

closing, got a opal-webpack-loader instead

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

No branches or pull requests

3 participants