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

Added caching of compiled build scripts. #859

Merged
merged 4 commits into from
Jul 14, 2015

Conversation

xavierzwirtz
Copy link
Contributor

Caches the script file to a compiled dll when first run. Uses crc32 to hash the script, prevents using old dll when the script file has changed. Also recursively follows #load.

Takes 3312 milliseconds to interpret script without saving cache.
Takes 3326 milliseconds to interpret script and save cache.
Takes 260 milliseconds to run cached dll.

Fixes #646, and makes #645 less needed.

@xavierzwirtz
Copy link
Contributor Author

Requires fsharp/fsharp-compiler-docs#365. It has been merged, don't know when it will be released.

@xavierzwirtz
Copy link
Contributor Author

Implementing support for #r'd DLLs looks to be all that's missing. Only thing that's keeping my build scripts from running. Will work on fixing that.

@xavierzwirtz
Copy link
Contributor Author

Added support for build scripts with #r and #I directives. This now runs build scripts generated by FSharp.ProjectScaffold.

@forki
Copy link
Member

forki commented Jul 13, 2015

very very cool. is this ready?`whyt about the appveyor build?


included.ShouldEqual(new string[] { "justname", "./relative/path", "C:/absolute/path" });
};
//It should_load_assembly =
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why is this commented?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was originally parsing out the included assemblies in the script files, that was the test for it. I switched to saving a list of all loaded assemblies paths and loading those on next run. Made the test not needed.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok so could you please remove these?
On Jul 13, 2015 18:36, "Xavier Zwirtz" notifications@github.com wrote:

In src/test/Test.FAKECore/FSIHelperSpecs.cs
#859 (comment):

  •            var newHash = FSIHelper.getScriptHash(scriptContents);
    
  •            hash.ShouldNotEqual(newHash);
    
  •        };
    
  •    It should_get_included_assemblies =
    
  •        () =>
    
  •        {
    
  •            var script =
    
  •                "#r \"justname\"\n" +
    
  •                "#r \"./relative/path\"\n" +
    
  •                "#r \"C:/absolute/path\"";
    
  •            var included = FSIHelper.getIncludedAssembly(script);
    
  •            included.ShouldEqual(new string[] { "justname", "./relative/path", "C:/absolute/path" });
    
  •        };
    
  •    //It should_load_assembly =
    

I was originally parsing out the included assemblies in the script files,
that was the test for it. I switched to saving a list of all loaded
assemblies paths and loading those on next run. Made the test not needed.


Reply to this email directly or view it on GitHub
https://github.com/fsharp/FAKE/pull/859/files#r34482490.

@xavierzwirtz
Copy link
Contributor Author

Removed that dead code.

@xavierzwirtz
Copy link
Contributor Author

The appveyor build seems to be swallowing the exception that occured. Any help with that?

@forki
Copy link
Member

forki commented Jul 14, 2015

image

if you scroll up a bit you can see the test failure

@forki forki merged commit 801eb9d into fsprojects:master Jul 14, 2015
@forki
Copy link
Member

forki commented Jul 14, 2015

I fixed the tests, but now I have issues with FAKE in fsreveal:

#I @"packages/FsReveal/fsreveal/"
#I @"packages/FAKE/tools/"
#I @"packages/Suave/lib/net40"

#r "FakeLib.dll"
#r "suave.dll"

#load "fsreveal.fsx"

image

I think we need to look in all paths.

@xavierzwirtz
Copy link
Contributor Author

Ah, didn't realize that #load took into account #I. I can add handling for that.

@forki
Copy link
Member

forki commented Jul 14, 2015

yes please send pull request. I can then update the FAKE 4.0 alpha package. I think this feature deserves it's own major version.

@xavierzwirtz
Copy link
Contributor Author

Agreed, it has enough potential weirdness hiding in it. Are the trace calls to output the state of the cache acceptable? I could not decide if they were meaningless noise when running FAKE in production.

@forki
Copy link
Member

forki commented Jul 14, 2015

I think we will put these to verbose mode before we release 4.0

@xavierzwirtz
Copy link
Contributor Author

#859 has the fixes. Now that I have it working though, FSReveal is not going to save the cache until you stop watching changes. Felt a little odd.

@forki
Copy link
Member

forki commented Jul 14, 2015

mhm. I don't think that's an issue. or is it?

@xavierzwirtz
Copy link
Contributor Author

Not a huge issue, if you hit a key it finishes and caches it.

Thanks for the help getting this pulled in.

On July 14, 2015 1:01:48 PM Steffen Forkmann notifications@github.com wrote:

mhm. I don't think that's an issue. or is it?


Reply to this email directly or view it on GitHub:
#859 (comment)

@xavierzwirtz xavierzwirtz deleted the cached-compile branch July 14, 2015 20:06
@forki
Copy link
Member

forki commented Jul 15, 2015

fake

@forki forki mentioned this pull request Jul 20, 2015
@forki
Copy link
Member

forki commented Jul 20, 2015

@xavierzwirtz it seems we need a page in the docs which describes what happens here. Could you please create such a page?

@xavierzwirtz
Copy link
Contributor Author

Created in #872.

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

Successfully merging this pull request may close these issues.

2 participants