Manually clean up paths.

[evanmcc]

Using code:set_path/1 with very large paths is very slow on larger
projects. On my mid-sized project, it seems to take around .4s per
call. Emulating the call with direct path removal (using
code:del_path/1) seems to be quite a lot faster.

[Vagabond]

This seems to shave about 30% off the time for a riak build. I've reviewed the code and tested it and I'm +1 on this change.

[Vagabond]

30% off the time for a build when everythng is already built, I should clarity.

[seth]

set_path does verification of the paths and read file data. e.g. here: https://github.com/erlang/otp/blob/maint/lib/kernel/src/code_server.erl#L777

so makes sense that manual cleanup via del_path is faster.

[ghost]

How about renaming the fun to cleanup_code_path/1, as this module is a collection of many unrelated funs?

[ghost]

Thanks @evanmcc. We don't (yet) have a test suite for code path changes, but the updated CONTRIBUTING guide requires tests for behavioral changes. So, we should try to write tests before we merge this.

[ghost]

Minor commit message issue: following CONTRIBUTING#commit-titlesummary, there should be no period in the title/summary.

[evanmcc]

I've addressed all of the concerns but adding the test. If someone sees an easy way to test this, I'm all ears. Otherwise I'll try to figure it out on Monday.

[ghost]

Thanks.

[ghost]

Minor typo: if someone -> If someone

[ferd]

Looking at the operations in code:add_path, this sounds relatively safe. If I get it right, set_path will manually re-scan all the paths in the list to make sure to extract .ez archives and whatnot, whereas delete_path checks nothing and add_path only checks the new path.

This sounds safe enough to me for a decent performance gain.

[ferd]

Sorry, I noticed that we never re-add paths. In the off-chance some paths were deleted somewhere in the code we set_path again with this patch. It seems slower than trying to re-add the missing ones one by one and hoping they also fall in the same order, but safer. I'm fine with it.

[evanmcc]

Can I get some clarification on what needs to happen to get this mergeable? A test and ??

[ferd]

I'm guessing tests and fixing the typo @Tuncer has seen in a comment. The rest seemed good to me, unless someone has something else to point out.

[Vagabond]

What would a test even look like, I don't even know why rebar resets paths like this, is it in case a task changes the paths somehow? Presumably via some plugin?

[evanmcc]

My plan was just to manually add and remove some paths and then test that they get properly reset. I have no idea how to test it as it would actually be used in rebar.

[ferd]

I would probably be fine testing the path changes done with code:set_path and the new method and making sure that they're equivalent.

[evanmcc]

Typo fixed, tests added, LC return explicitly ignored.

[Vagabond]

The eunit tests are calling teardown without setting the path and teardown() calls set_cwd(".."), which is silly but hard to fix right now.

Here's a patch to make the tests green:

 code_path_test_() ->
     [{"Ensuring that fast code path cleanup is correct for adds",
-     setup, fun make_tmp_dir/0, fun teardown/1,
+     setup, fun make_tmp_dir/0,
+      fun(_) -> remove_tmp_dir() end,
      fun() ->
              OPath = code:get_path(),
              PathZ = ?TMP_DIR ++ "some_path",

[Vagabond]

Merging this.