search

timocov / dts-bundle-generator

A tool to generate a single bundle of dts with types tree-shaking
MIT License
762 stars 40 forks source link

Investigation: Use of `Context.Tag` from `effect` causes 4x slowdown #302

Closed leighman closed 6 months ago

leighman commented 8 months ago

Bug report

Will work on a smaller reproduction but within our project we've done a bisect and narrowed it down to the use of 

+export const DBTransactionClient = Context.Tag<DBTransactionClient>()

where +import { Context } from 'effect'

Can't see anything obvious in terms of types that should cause the problem.

Would appreciate any suggestions on what this could be/how to investigate further.

timocov commented 8 months ago

Do you observe the same effect for compilation? Or it is just dts-bundle-generator?

timocov commented 8 months ago

It seems I found some potential points of improvements, I'll keep you posted here. Meanwhile if you have any additional information, let me know.

timocov commented 8 months ago

I created a PR with some improvements https://github.com/timocov/dts-bundle-generator/pull/303. @leighman If you can validate it with your scenario and provide some feedback here it would be much appreciated (simply checkout the branch, run npm run verify, then npm link, and then finally npm link dts-bundle-generator in your project, then run commands as usual). Let me know if you have any questions.

leighman commented 8 months ago

Nice. Will give it a try in the next few days hopefully. There was no obvious corresponding effect on tsc compile time but we've had effect library in for a while and have been using other parts of it, etc, so it's possible that there was some effect at the same time.

leighman commented 8 months ago

Perhaps I did something wrong but the PR branch seems to unfortunately have no effect over all. Maybe there's another contributor

timocov commented 8 months ago

@leighman What steps did you do to check it? If you can run the tool with the nodejs profiler enabled and upload profiles before/after it might help

leighman commented 8 months ago

@timocov I followed the steps in your comment and it seemed to be linked correctly. Run the profiler on version we were using and on the PR branch? Is that what you mean by before/after?

timocov commented 8 months ago

@leighman I believe I meant before adding that line/after on the version in PR, but probably it doesn't make much sense so I think it's fine to start with just "on your code with the tool from PR"

timocov commented 7 months ago

I'm planning to make a release with changes we currently have to see if they made any improvements for your case. But providing the profiler profile would be much appreciated and can help to investigate it further.

timocov commented 7 months ago

@leighman do you have any updates?

leighman commented 7 months ago

I'm afraid we ended up just doing a bundle then committing the files and for now we will maintain them manually so won't really be testing regularly.

timocov commented 6 months ago

@leighman Thanks for the update. I'm going to close the issue in this case. Please let me know if you will have any other update or an opportunity to provide more data on this. In any case, feel free to ping me here or cut a new issue with other details.

timocov commented 6 months ago

The fix has been published in 9.4.0 version.