-
Notifications
You must be signed in to change notification settings - Fork 6
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
Cache riml_includes between compile_files #16
Comments
This is a great idea, thanks! Also, it's easy to implement 😄. I am going to have to wrap a mutex around the cache though, with the way things work now. How much faster is compilation right now with I'm also going to use I'll let you know when this is ready :) |
Quite a bit. It eliminates the boot time of ruby. And it's saving is proportional to the size of the suite. 😄
I suspect I'm not seeing this as I am passing in one file at a time to |
I noticed a boost in compile times, Looks like this is working in master? |
Hey @dsawardekar, this is indeed working in master, I'm glad you noticed a speed increase 😄 Thanks again for the suggestion. |
Thanks. I will update Speckle, this is an awesome update! :) |
@luke-gru About this update, It shaved about 20 seconds of the build time of Portkey! So this was a really nice update. Not trying to be greedy, But I think you can do even better! The time taken for a complete compile of Portkey is about 5 seconds. About 1 second for Yet the whole test suite compilation takes about 40-50 seconds. Some of the tests include the entire app with In contrast running the test suite in Speckle zips by in under 1 second, compiling about 10+ files. Speckle does not have any nested includes. Does the the |
Hmm, this is interesting, as I suspected it did this already, but I didn't benchmark anything at all, so I kind of tested it the lazy way. If you take a look at https://github.com/luke-gru/riml/blob/master/test/integration/riml_commands/compiler_test.rb#L287, the mocked cache object I'm using during testing is apparently called with the nested includes, but something must be wrong if it's taking so long to compile. I'll take a look at this, and make sure to add some benchmarks to the testing process, as the compile times are a little worrying right now, 40-50 seconds is pretty bad. I'm sure we can do better than that 😄 EDIT: I didn't know about Portkey! Looks awesome, I'm going to check it out. |
Would assembly after compilation take too much times? If you run You can even see this isolated with, $ speckle spec/controllers/commands/alternate_file_command_spec.riml spec/controllers/commands/related_file_command_spec.riml These are E2E tests. Both these specs are very similar in size. Compiling the alternate command should compile the entire app. The related command should then be just a quick cache lookup + the tests in the file itself which is only a handful of methods. The related command still takes a 2-3 seconds to compile. Adding a number of these commands is what is adding up to 40+ seconds in the whole suite. |
@luke-gru Compile times now down to 12 seconds in Portkey in master. Awesome work! Are you certain there were no |
haha, thanks. No |
Neat stuff. I'm glad I kept asking for more. At 12 seconds it's around the same time it takes the full suite to run. Can't ask for more now! :) |
I just updated to 0.3.2, and it helped me figure out a bunch of dups I had been using without realizing. Thanks for that!
I also made an improvement to
Speckle
to useRiml.compile_files
directly instead ofbundle exec riml
. This speeds up compilation quite a bit, and leads me to this feature request.I have quite a few tests that do a lot of
riml_include 'foo'
style includes. For instance I have a couple of classes in files,log_helpers.riml
anddelegate.riml
anddsl.riml
. These get include in nearly every spec at the top like so,For simpler files this is not a big issue but for E2E tests the whole plugin is included which makes compile times quite slow.
Would it be possible to cache the result of a
riml_include
for the duration of a riml compile session?To clarify further, say I have E2E tests to run with includes like,
In controller_a_spec.riml
In controller_b_spec.riml
Now say when I run,
The compiler should compile
controller_a_spec.riml
and it's includesa, b and c
. But forcontroller_b_spec.riml
, it should just return the previously compiled a, b, and c includes, from in-memory cache.Thus compiling
controller_b_spec.riml
does not incur the cost of compilinga, b and c
. It only has to assemble the rest of the contents ofcontroller_b_spec.riml
This feature would speed up compilation by an order of magnitude for
Speckle
, by at least a factor of 5!The text was updated successfully, but these errors were encountered: