Skip to content
This repository has been archived by the owner on May 6, 2022. It is now read-only.

Adds RFC proposal for caching generated assembly #2

Closed
wants to merge 1 commit into from
Closed

Adds RFC proposal for caching generated assembly #2

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
84 changes: 84 additions & 0 deletions text/0000-cache-generated-assembly.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,84 @@
- Start Date: 2018-10-31
- RFC PR: (leave this empty)
- Cake Issue: (leave this empty)
- Affected Areas: Cake, Cake.Core, Cake.Scripting

## Summary
[summary]: #summary

Include in Cake the ability to cache the generated assembly, so that this
can be used when the underlying script hasn't changed.

## Motivation
[motivation]: #motivation

While for most people, the speed in which Cake executes is perfectly fine,
however, there have been those that suggest that the compilation time is a
burden that they don't want to spend. Especially in situations where the build
script itself isn't changing, but lots of builds are happening in succession.

Caching the generated assembly so that it can be used over and over again, without
the need to compile each time, will save some workloads loads of time.

## Detailed design
[detailed-design]: #detailed-design

### Changes to configuration

A new entry will be added into the cake.config file to control the location of
the cached assemblies. This should default to a `cache` folder within the Cake
Tools folder. The end user can then control this location in the normal way.

### Changes to Cake Arguments

Two new arguments will be added to the Cake Client.

* Cache - If set true the compiled script will be cached
Copy link

Choose a reason for hiding this comment

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

I think this could (should) be true by default. Maybe there can be an additional check to check modification time of files against the build time of the cached assembly to determine whether a new build is required.

Copy link

Choose a reason for hiding this comment

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

They wanted this to be opt-in so that is why it was defaulted to false. There is an additional step to determine if the cached assembly is valid. A hash is taken of the final script and that is used to determine if the cache is valid or not.

* Recompile - If set true it will force a recompile of the script even if a valid cached output exists.

These will be added to the CakeOptions class, and set based on the values which
are parsed in the ArgumentParser.

### Changes to Cake.Scripting

The majority of the work for this RFC will be done in the RoslynScriptSession class,
although a change will be needed to the RoslynScriptEngine to pass in the CakeConfiguration
instance, to inspect the configured values.

Within RoslynScriptSession, a check will be made to see whether caching of the script
is enabled. If it is, and a cached assembly is already present, and we are not being
asked to force the re-compilation of the script, then we should go ahead and run that
assembly, rather than compiling the script. Within this section, we should also check
the timestamps of both the cached assembly and the script. If the script is newer than
the cached version should not be used, and instead, re-generated.

If we are being asked to cache the script, and the assembly doesn't exist, we should
generated the assembly and then execute it. Otherwise, run the script as we currently
do.

## How we teach this
[how-we-teach-this]: #how-we-teach-this

[Documentation](https://github.com/cake-build/website/blob/develop/input/docs/cli/usage.md)
will need to be updated to reflect the new arguments that this adds to Cake.

The Help system which is displayed on the Command Line will also need to be
updated to include the new arguments.

## Drawbacks
[drawbacks]: #drawbacks

This will add some complexity to Cake, as I don't think that this is something
that will be enabled by default, but rather something that people will have to
opt into. That in itself will cause some initial confusion, and likely some
misunderstanding.

## Rationale and alternatives
[rationale-and-alternatives]: #rationale-and-alternatives

We could keep things as they are.

## Unresolved questions
[unresolved-questions]: #unresolved-questions

Don't think there are any.
Copy link
Member

Choose a reason for hiding this comment

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

Caching compilation is not alone enough. We must also cache registered tools resolved via #tool directive. Addins (and their dependencies) must also be loaded into app domain / load context in same order as the first time when resolved via #addin directive.

Copy link

Choose a reason for hiding this comment

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

Everything still runs the same as previously. The script itself is still processed by cake to pickup the #addin's and #tool's. Once this is done and the complete script is generated this is where cake would compile the script to run. At this point it checks to see if a valid cached compile scripted exists. If it does then it runs the cached script.