Remove Kernel#caller patches
wied03 opened this issue · 6 comments
Karma-opal-rspec is now using stacktrace.js:
This issue should now:
- Remove the
Kernel#calleroverride in opal-rspec and just put in pure, raw JS stack traces only when a test fails and point people elsewhere (wherever this github issue gets done) to use source maps. Better yet, don't include any location info to avoid the overhead (see next bullet) - #57 might be a more performant way of gathering metadata for every test
@elia - Do you think this belongs in a separate GEM? These Bower dependencies can be pulled in via rails-assets (I use that to represent Bower deps as GEMs on my project) but I think this probably belongs in a separate GEM.
@wied03 I'm quite sure this should be optional, not necessarily by using another gem but one shouldn't be forced in. Although an argument in favor of an external gem can be made by pointing out that the gem could be independent from RSpec and just provide source-mapped backtraces to opal on nodejs.
Some Kernel#caller discussion here - opal/opal#894
It probably needs to be removed anyways because it's poorly tested and brittle, but removing this code in the kernel_caller_fix branch didn't improve performance that much:
Test was conducted using time bundle exec rake opal_specs. Around 40 of the 140 tests are expected to fail so they will have exceptions regardless. I would have expected the other 100 tests that pass to improve these results more than they did.
Just throwing it in here for posterity that the majority of time spent in RSpec overhead (that is, not the actual specs themselves) was due to break calls in Enumerable.
Considering the results in opal/opal#1450, you can probably multiply any benefit gained by at least 4x. Possibly a lot more, considering that timing the Rake task measures time between command prompts, which includes loading node, Opal, and RSpec. This is a huge portion of execution time, too. For example, Clearwater specs take about 8 seconds on my machine, but nearly all of that is load time.
A better benchmark would probably be the execution time of the specs only.