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

Generate script: Don't write .cc or .h files if unchanged #1091

Merged
merged 4 commits into from
Feb 7, 2017

Conversation

smith-kyle
Copy link
Contributor

The generation script now checks whether the file has changed before writing it to /src or /include. This is to improve compilation time when testing changes to generated code.

It works by creating a /temp directory, writing the generated code to /temp/src and /temp/include, then syncing those folders with /src and /include by deleting files that no longer exist and copying files that have changed or been added since the last code generation. Finally the /temp directory is deleted

If /src and /include don't exist (i.e. it's the first time the generation script has been run), /temp/src will be copied to /src and /temp/include will be copied to /include.

@smith-kyle smith-kyle force-pushed the improve-generate branch 2 times, most recently from 4a47146 to 4c1a277 Compare July 26, 2016 23:51
@maxkorp
Copy link
Collaborator

maxkorp commented Aug 8, 2016

You realize that using node-gyp build instead of node-gyp rebuild already takes care of this, right?

@implausible
Copy link
Member

@maxkorp i think this fixes the issue where you make a change to the templates / add definition / add supplement, and you need to run generate, again. This stops the generate script from overwriting unchanged files. That way if you run node-gyp build, it will only build the files that have changed, rather than compile everything @smith-kyle am I right here?

@maxkorp
Copy link
Collaborator

maxkorp commented Aug 12, 2016

Ah interesting, my bad. Lemme play with it a bit, that could be useful then 👍.

@maxkorp
Copy link
Collaborator

maxkorp commented Aug 18, 2016

So, the overall recompile time without this is still fairly small, the bulk of the overall compile time is openssl and that isn't affected in this case (and the second largest is libgit2, which is also unaffected).

That said, if this is a big deal to you guys I'm cool with it. Any reason we don't use os.tmpdir though?

@smith-kyle
Copy link
Contributor Author

smith-kyle commented Aug 23, 2016

Still relatively small but compilation is going from ~90 seconds to < 5 seconds if you've only changed a couple generated files.

This is the first I'm hearing about os.tmpdir. I don't know much about temp dir etiquette but this seems like a good use for it. Thanks for the suggestion @maxkorp I'll switch it over.

@maxkorp
Copy link
Collaborator

maxkorp commented Aug 23, 2016

Awesome. Thanks a pretty big change, so I can dig it 👍

@smith-kyle smith-kyle changed the title Generate script: Don't write .cc or .h files if unchanged [WIP] Generate script: Don't write .cc or .h files if unchanged Aug 29, 2016
@smith-kyle smith-kyle changed the title [WIP] Generate script: Don't write .cc or .h files if unchanged Generate script: Don't write .cc or .h files if unchanged Nov 13, 2016
@smith-kyle
Copy link
Contributor Author

@maxkorp I've (finally) moved this to use os.tempdir. It's ready for review. Not sure why appveryor is dying

@maxkorp
Copy link
Collaborator

maxkorp commented Jan 30, 2017

I don't have push access to your repo to rebase this, so you gotta rebase this, but it's G2G man.

Kyle Smith added 4 commits January 30, 2017 13:23
The generation script now checks whether the file has changed before writing it to `/src` or `/include`. This is to improve compilation time when testing changes to generated code.

It works by creating a `/temp` directory, writing the generated code to `/temp/src` and `/temp/include`, then syncing those folders with `/src` and `/include` by deleting files  that no longer exist and copying files that have changed or been added since the last code generation. Finally the `/temp` directory is deleted

If `/src` and `/include` don't exist (i.e. it's the first time the generation script has been run), `/temp/src` will be copied to `/src` and `/temp/include` will be copied to `/include`.
@smith-kyle
Copy link
Contributor Author

Rebased 👍

@johnhaley81
Copy link
Collaborator

Tested. Looks good, works good. Thanks @smith-kyle!

@johnhaley81 johnhaley81 merged commit 74a7e07 into nodegit:master Feb 7, 2017
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