SketchUp / api-issue-tracker

Public issue tracker for the SketchUp and LayOut's APIs
https://developer.sketchup.com/
38 stars 10 forks source link

Sketchup.require much slower than require #309

Open thomthom opened 5 years ago

thomthom commented 5 years ago

With a complex extension like Vertex Tools I'm seeing significant performance impact of Sketchup.require over require - even when loading.rbfiles. Had it been.rbsor.rbe` I could have understood the decryption having an overhead, but with plain Ruby files I would not expect any significant difference.

Reload method for my extension:

module TT::Plugins::VertexTools2

  # Only works for RB files - not scrambled RBS version.
  #
  # @example
  #   TT::Plugins::VertexTools2.reload
  #
  # @return [Integer]
  # noinspection RubyGlobalVariableNamingConvention
  def self.reload
    t = Time.now
    original_verbose = $VERBOSE
    $VERBOSE = nil
    # Deactivate Vertex Tools before reloading.
    model = Sketchup.active_model
    if model
      editor = PLUGIN::Editor.get
      model.select_tool(nil) if editor && editor.active?
    end
    # Reload all the Ruby files.
    x = Dir.glob( File.join(PATH, '**/*.rb') ).each { |file|
      # noinspection RubyResolve
      load file
    }
    ObjectSpace.each_object(PLUGIN::Editor) { |editor|
      editor.release
    }
    NotificationCenter.default.reset
    Log.info "reload: #{(Time.now - t).round(3)}s"
    GC.start
    Log.info "reload + GC: #{(Time.now - t).round(3)}s"
    x.length
  ensure
    $VERBOSE = original_verbose
  end

end

An invocation of the reload method yields: (This is loading only .rb files)

TT::Plugins::VertexTools2.reload
reload: 1.63s
reload + GC: 1.671s
137

That's slow - considering this will happen upon SketchUp starts as it loads extensions. (Unless I change to lazy-load files.)

If I rewire Sketchup.require to require...

module Sketchup
  def self.require(*args)
    Kernel.require(*args)
  end
end

...and then run reload:

TT::Plugins::VertexTools2.reload
reload: 0.237s
reload + GC: 0.28s
137

That's a significant differences which warrant an investigation.

DanRathbun commented 5 years ago

I do not see a call Kernel#require or Sketchup::require in the snippet above ?

thomthom commented 5 years ago

No, not in the reload script, but each .rb file it loads has a number of Sketchup.require statements.

Might be inefficient lookup of already required files.

taustin73 commented 5 years ago

SU-44287