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

Add support for incremental compile #612

Merged
merged 3 commits into from
Aug 2, 2022
Merged

Conversation

yissachar
Copy link
Collaborator

No description provided.

Copy link
Collaborator

@swankjesse swankjesse left a comment

Choose a reason for hiding this comment

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

👏🏻 🎉 LOVE THIS

manifestSigner = manifestSigner
)
if (inputChanges.isIncremental) {
val filterByChangeType: ((ChangeType) -> Boolean) -> List<File> = { filter ->
Copy link
Collaborator

Choose a reason for hiding this comment

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

ooooh this is a tricky signature

Copy link
Collaborator

Choose a reason for hiding this comment

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

Could this be a fun instead of a val ?

      fun filesByChangeType(changeType: ChangeType): List<File> {
        inputChanges.getFileChanges(inputDir)
          .filter { filter(it.changeType) }
          .map { outputDir.file(it.normalizedPath).get().asFile }
      }

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yeah, this is the problem with incremental refactors 😅 - definitely should be a fun instead.

writeManifest(outputDir, mainFunction, mainModuleId, manifestSigner, modules)
}

fun incrementalCompile(
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is rad!

}.quickjsBytecode.toByteArray())
}

assertZiplineIncrementalCompile("$rootProject/base",
Copy link
Collaborator

Choose a reason for hiding this comment

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

neato

hashingSink.hash
}
val jsFiles = getJsFiles(inputDir.listFiles()!!.asList())
jsFiles.forEach { jsFile -> compileSingleFile(jsFile, outputDir, modules) }
Copy link
Collaborator

Choose a reason for hiding this comment

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

do we need to skip non-.js files?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

getJsFiles handles that

Copy link
Collaborator

@JakeWharton JakeWharton left a comment

Choose a reason for hiding this comment

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

There's probably a long tail of smaller optimizations such as avoiding intermediate collections, minimizing string allocation, etc.

This is a huge improvement as-is though!

Comment on lines +94 to +97
// Clean up dir from previous runs
if (outputDir.exists()) {
outputDir.deleteRecursively()
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should probably use a JUnit temporary directory rule here to ensure a clear slate

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Maybe I'll do this as a separate PR, since really we should do this for all test, not just the incremental ones.

Copy link
Collaborator

@adrw adrw left a comment

Choose a reason for hiding this comment

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

LG

yissachar and others added 2 commits August 2, 2022 17:23
…ZiplineCompileTask.kt

Co-authored-by: Jake Wharton <jw@squareup.com>
…ZiplineCompiler.kt

Co-authored-by: Jake Wharton <jw@squareup.com>
@yissachar yissachar merged commit 7eb3169 into trunk Aug 2, 2022
@yissachar yissachar deleted the yissachar/incremental-compile branch August 2, 2022 23:11
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.

4 participants