Skip to content

Conversation

@jasonlyu123
Copy link
Member

With the project version, typescript language service would skip rebuilding program when the version is the same.
https://github.com/microsoft/TypeScript/blob/3ef3cdddb338b8f0e6a2c5971d255390b68654ac/src/services/services.ts#L1290

Even though in most cases the rebuild doesn't affect performance very much because most of the state is reused. But update import would be drastically impacted by this. As in its deep call hierarchy, it would try to check if the program object is up-to-date multiple times. So this small difference cumulated to be a big time.

This mostly benefits update import and svelte-check. One of my projects improved check time from 30 seconds to 19 seconds. The same project previously can't even finish update import within 5 minutes. And it's now gone down to less than 1 minute.

One other thing I want to discuss. I also found that if I remove the custom readFile implement with getSnapshot the updated import of the previously mention project even went down to less than 5 seconds. From what I found, the reason is that typescript uses readFile to read some source map files (.map) and their source file. There's a certain node module ship declaration files with sourcemap comment and the src folder for some reason. Because getSnapshot add files to the compilation list. It would cause typescript to constantly rebuild the program during update import. Wondering what should we do about this. Should we remove the custom readFile implementation? Or we can replace it with another way to cache the read file result?

As these are also being covered by globalSnapshotManager change event
Copy link
Member

@dummdidumm dummdidumm left a comment

Choose a reason for hiding this comment

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

This sounds great, thank you for looking into this!

About readFile: Do you mean removing the logic of calling getSnapshot inside svelteSys.ts? If so, what would we replace it with? I think right now we need that hook because else we can't intercept Svelte file reads that happen as a result of file import crawls. In the TypeScript plugin we do it a little differently, maybe we can adjust that here, too.

@jasonlyu123
Copy link
Member Author

Besides removing the logic of calling getSnapshot in svelteSys.readFile. One alternative is not to call it in specific situations, like It's a ts file in the node_modules folder. Or only call it when we know it's a source file like when it's a .ts, .js, .svelte (or .json even).

I'll do more research on the readFile problem. And do it in another PR. This PR should be enough for most of the users. The readFile problem only occurs when there's a .d.ts file with source map comment.

@jasonlyu123 jasonlyu123 merged commit 5d7f676 into sveltejs:master Nov 12, 2021
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.

2 participants