Caching swift templates

[ilyapuchka]

No description provided.

[krzysztofzablocki]

@kzaher if you have time, it would be better for you to review it since you are the author :)

[kzaher]

Hi @krzysztofzablocki , @ilyapuchka ,

I've made some suggestions. Hope it will help.

[kzaher]

I think it would be cool to make this something like.

extension SwiftTemplate {
     func build(buildDir: Path, cachePath: Path?) throws -> Path {
            return lazy("cached_binaries", source)  { () -> Data in
                    // build binary content and return it here
            }
     }
}

We could separate this method into generic caching functionality (lazy loading) and generating part.

I also think this entire method could be simplified.

If we change SwiftTemplate like I've suggested below, then SwiftTemplate becomes only a value type.
We can use source hash as a path to binary, and just check does it exist or not (inside lazy method).

Right now it seems to me we only use single path for all templates. It should probably be let cachedBinaryFile = cachePath.map({ $0 + Path("bin") + source.hash }).

We also wouldn't need this part of code

+        if let cachedTemplateFile = cachedTemplateFile,
 +            let cachedBinaryFile = cachedBinaryFile {
 +            do {
 +                let data = NSKeyedArchiver.archivedData(withRootObject: template)
 +                try cachedTemplateFile.write(data)
 +                try binaryFile.copy(cachedBinaryFile)
 +            } catch {
 +                try? cachePath?.delete()
 +            }
 +        }

then, because we don't store template. SwiftTemplate should be momentarily because it only depends on a couple of O(n) complexity string operations.

[ilyapuchka]

There is a separate path for each template, named by template name, so paths are fine now.
I like your idea about using hash as a file name, instead of storing it in archive. It will save us from unneeded unarchiving operations. I guess we should do that for parser results cache.

Regarding your last comment we need to store at least a binary, because it takes most of the time to compile generated code. It will be faster when runtime files will be precompiled but it will still take few seconds. With cached binary we just run it. If we combine it with your first suggestion to use hash as a name we can avoid storing main.swift and there will be no need in archiving and unarchiving its content at all

[kzaher]

Let me try to restate my suggestion :)

class SwiftTemplate {
     let generatedCode: String
     let cachePath: String?
}

let t = SwiftTemplate(templatePath, cachePath) // parsing template and generating source code is done here in convenience init, it's fast, so no caching is needed

func render(....) {

    let binaryPath = Path(cacheDir + t.generatedCode.hash)
    if !binaryPath.exists {
         Path.move(t.compile(buildDir), binaryPath)  // store it in cache
    }
 
    /// serialize everything and run binaryPath  
}

[ilyapuchka]

exactly what I thought of 👍

[kzaher]

Do we need var? Actually, do we need sourcePath at all? We could only use it in convenience initializer that is made for parsing.

[kzaher]

I'm not sure do we need this, this could be derived from code IMHO.

[kzaher]

This could be transformed into convenience init that only takes path. It seems to me that cachePath is only related with loadOrBuild method, but I've given a suggestion to transform it into an extension on SwiftTemplate.

func build(buildDir: Path, cachePath: Path?) throws -> Path {

This would separate SwiftTemplate which could be considered as a value type with only let code: String.

I think it would make sense to extend func render(types: Types, arguments: [String: NSObject]) with cachePath so we could use build inside of it.

Alternative would be to also store let cachePath: String? and let source: String in SwiftTemplate, but I would still call build method inside render and make this just a dummy value type, although it's a NSObject.

[ilyapuchka]

For me it feels that it's better to keep things separate, parsing is parsing, rendering is rendering. But I'll keep looking in how to organise code better, that's not the final version.

[SourceryBot]

	1 Warning
⚠️	Big PR

Generated by 🚫 danger

[ilyapuchka]

@kzaher @krzysztofzablocki any ideas how to make swiftc link frameworks? I've tried -F and -framework but it feels like they have no effect. Maybe I miss some other flags to make it work, but I could not find any inspecting Xcode build commands.

I was able to build template with SPM swift build. But this requires not a framework binary but module build artefacts.

[ilyapuchka]

@krzysztofzablocki @kzaher can you please test my latest changes locally? I think it should work much faster now 🎉

[ilyapuchka]

Kudos to @kylef for this helpful tip 🙇

[ilyapuchka]

@krzysztofzablocki have some troubles here making SPM build work again. First it made me to move all the files to the single folder... Then I added a framework to targets in Package.swift, like this:

targets: [
        Target(name: "sourcery", dependencies: ["SourceryFramework"]),
        Target(name: "SourceryFramework"),
    ]

And this gives error, descriptive as any Apple error:

error: these referenced modules could not be found: SourceryFramework
fix: reference only valid modules

[kzaher]

Hi @ilyapuchka ,

I believe for you to be able to write something like

targets: [
        Target(name: "sourcery", dependencies: ["SourceryFramework"]),
        Target(name: "SourceryFramework"),
    ]

You need to have SourceryFramework named directory under Sources that contains all of *.swift files in a flat structure, no subfolders, no other files.

Now I see this

I also get some other compiler errors while trying to run the tests on 172ec96.

[ilyapuchka]

That's exactly what I did, tried both with simlynk (like for sourcery module) and just copying all the sources directly. I also have fixed compiler errors after merge. I've push all of that, if you have a chance to look at it deeper would be cool

[kzaher]

@ilyapuchka now I see

Doesn't look like this will compile 😀 Perhaps symlink gone wrong 😀 ¯\_(ツ)_/¯

I can probably hack a script that rebuilds this hierarchy if you like. We do this for RxSwift. Not the prettiest solution, but works :)

[ilyapuchka]

That was actually an issue, with proper symlink it works 🎉

[kzaher]

Ha, interesting. I think those subfolders might be treated as submodules. Probably documented somewhere ¯\_(ツ)_/¯

Just one more thing :)

[ilyapuchka]

Yes, if there is a folder in Sources with no main.swift it's treated as a framework module.
Regarding the error that you get you probably need to update yaml (I guess you use it as you don't provide any cli params, right?) file as the paths now are relative to its location

[kzaher]

I was trying to get it to print help page so I can figure out the arguments :)

[ilyapuchka]

./.build/debug/sourcery --help will do that.
There is in fact configuration file checked in already and by default we look at the current path and if there is config file we ignore cli arguments. Maybe we should change that and always use provided cli arguments. I've updated this config any way, so now it should work with just ./.build/debug/sourcery after you remove ejs template ofc.
Can you take another look at the changes when you have time? I'd like to work on some other features for swift templates like #275 or #243 but it would be better if they will be done on top of that to avoid more conflicts.

[ilyapuchka]

@krzysztofzablocki can we merge this? 🙏🏻

[krzysztofzablocki]

go ahead, just squash it 👍