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

feat: Bump ts poet for perf increase #714

Merged
merged 1 commit into from
Nov 26, 2022

Conversation

gwer
Copy link
Contributor

@gwer gwer commented Nov 26, 2022

Pulling ts-poet updates. I ran the tests locally and they are still green.

@stephenh
Copy link
Owner

Great, thanks!

Fwiw is the cost of calling dprint a large remaining hot spot for you?

If so, ts-poet has a ~cute optimization to use the hash of pre-formatted content to avoid calling dprint just to realize the new output vs. old output on disk hasn't changed.

The logic is handled by this saveFiles method:

https://github.com/stephenh/ts-poet/blob/585fc389a822eb909ddfeed8b30592024d4eb899/src/saveFiles.ts#L46

As a disclaimer, dprint is quick, so I'm not sure if this is worth while or not; I've got another ORM project using this saveFiles method and it seems to save ~half a second for ~150 files that are ~30k LOC combined. So, nice, but not super-noticeable for that specific project.

Also to leverage saveFiles's conditional behavior from ts-proto, we'd have to stop sending files unconditionally back (already formatted) in the CodeGeneratorResponse and maybe just have ts-proto's plugin.ts write the files directly to disk. Which, dunno, should be fine? But is probably not what tooling like buf/protoc expect.

Anyway, just mentioning that as an idea if you wanted to keep optimizing things. But there could definitely be other things to fix/optimize as well.

Thanks again!

@stephenh stephenh merged commit 0068dc8 into stephenh:main Nov 26, 2022
stephenh pushed a commit that referenced this pull request Nov 26, 2022
# [1.135.0](v1.134.0...v1.135.0) (2022-11-26)

### Features

* Bump ts poet for perf increase ([#714](#714)) ([0068dc8](0068dc8))
@stephenh
Copy link
Owner

🎉 This PR is included in version 1.135.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

@gwer
Copy link
Contributor Author

gwer commented Nov 26, 2022

Hmm, I just keep the hashes of the input files at the time of the last generation. Thus, if the inputs have not changed, then protoc does not start at all.

But this logic is implemented in the protoc startup script. I'm not sure that it can be correctly implemented in the plugin.

@stephenh
Copy link
Owner

Ah nice! Agree that sounds even better.

I guess for my ORM use case, one of the inputs is the database schema, so its harder to have that has an input to hash. That and handling incremental changes, i.e. 99 tables are all the same, but 1 is different, so the overall schema hash would have changed, but only a handful of individual output files will change. 🤔

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants