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

Stop using temp dir for GWT compile #4187

Merged
merged 2 commits into from
Jul 17, 2023
Merged

Conversation

niloc132
Copy link
Member

@niloc132 niloc132 commented Jul 14, 2023

The file /tmp/gwtWork/io.deephaven.web.DeephavenApi/compiler/permutation-0.js is in fact left behind after running ./gradlew prepareCompose. Since the file has always the same path regarding of user or user session, this mean two users on the same machine won't be able to build without clashing with each other (because the second user to run won't be able to overwrite the first user's file in /tmp), and for the same user, two parallel executions could clash.

jcferretti
jcferretti previously approved these changes Jul 14, 2023
@niloc132
Copy link
Member Author

As discussed in slack, i'm going to read a bit more on seeing about preventing some of this from being written to disk, and adding a deleteOnExit for that path.

@@ -84,7 +86,7 @@ class GwtTools {
/** The level of logging detail (ERROR, WARN, INFO, TRACE, DEBUG, SPAM, ALL) */
logLevel = "INFO"
/** The workDir, where we'll save gwt unitcache. Use /tmp to avoid cluttering jenkins caches*/
workDir = new File(System.getProperty("java.io.tmpdir"), "gwtWork")
workDir = Files.createTempDirectory("gradleGwtWork").toFile()
Copy link
Member

Choose a reason for hiding this comment

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

Doesn't a new directory defeat the purpose of a unit cache?

You should probably just put it into build/gwtWork and then it would go away on a clean

Copy link
Member Author

Choose a reason for hiding this comment

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

The unit cache is a different argument, this is the work dir which is mostly, but not entirely, cleaned up at the end of each build. Work dir holds stuff like incremental output (disabled by default for prod builds), intermediate linker output... some serialized precompile output to pass to out-of-process workers. It looks like there is a deleteOnExit for nearly everything, but apparently not for the dir itself, or the the permutation-N.js file (intermediate linker output). For these reasons, I think it probably makes more sense to remove the workDir param entirely - after a successful build, the full contents is less than 9kb for DHC, 14kb for DHE for just one file each.

The unit cache is disabled by default, and has to be explicitly given a directory to work in via system property gwt.persistentunitcachedir. If not provided, it will default to a sibling directory to the war dir.

The gradle plugin selected for GWT in our projects unconditionally puts the unit cache dir in the build dir:
https://github.com/esoco/gwt-gradle-plugin/blame/dca725f60a7e75dc9de11ea98d5c07d7f566a842/src/main/java/de/esoco/gwt/gradle/command/CompileCommand.java#L53

The unit cache might make sense to override, a dozen or so files, 120kb, but it isn't immediately obvious how we would do that.

@niloc132
Copy link
Member Author

I've updated the code changes to remove the work dir setting itself (since the output is tiny, and let's us sidestep these concerns entirely), and turn off the SOYC html report (so as to not write unnecessary/unused details to disk that would need to be cached).

@niloc132 niloc132 enabled auto-merge (squash) July 14, 2023 15:40
@devinrsmith devinrsmith added this to the July 2023 milestone Jul 17, 2023
@niloc132 niloc132 changed the title Ensure a new temp dir each build for gwt compile Stop using temp dir for GWT compile Jul 17, 2023
@niloc132 niloc132 merged commit abe8ff4 into deephaven:main Jul 17, 2023
@github-actions github-actions bot locked and limited conversation to collaborators Jul 17, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants