speedup gx-go rw/uw #38
Conversation
(large speedup)
So I don't have to explicitly import every single package that my dependencies import. If we're going to make this change, we need to flag every time we encounter a dependency we don't have a mapping for. |
rewrite/rewrite.go
Outdated
| @@ -107,13 +108,21 @@ func rewriteImportsInFile(fi string, rw func(string) string, rwLock *sync.Mutex) | |||
| return nil | |||
| } | |||
|
|
|||
| offset := file.End() | |||
| for i, d := range file.Comments { | |||
There was a problem hiding this comment.
Go ends up picking up an extra comment or two after the imports for some reason. Also, don't merge this yet. That commit appears to have broken something.
I already assumed this was a bug and fixed cases like this everywhere I found them.
It just feels a bit implicit. Also, it's probably a bit bug prone: we can (and unfortunately often do) import multiple copies of the same dependency. In this case, gx-go will just choose one.make |
Ah, you mean in the code? Yes, we will. Also, it appears that we have some packages we'll have to fix. |
|
Mmm. 50x faster. Unfortunately, you were right about the transitive dependency issue so I'll have to fix those first. |
|
I kinda intended it as a feature initially. Makes it so if i import foo that uses bar, I don't have to explicitly import bar to pass a bar.Cat to foo.GetAnimalNoise |
|
I get that, but its leaky. Even changing an internal dependency will break something. However, for now, I'll add that back. I still get a nice speedup by memoizing package.json files instead of reparsing them. |
|
So, really, finding rewrites recursively wasn't the performance issue. The performance issue was re-traversing parts of the DAG. @whyrusleeping this should work now. |
|
This also appears to reduce a fresh |
|
Okay, so we're switching to only rewriting the imports here. That sounds great to me. This whole bit with writing to a buffer multiple times, then writing to a temp file and then moving it over the top of the original seems a bit hectic... but it works, and its fast as hell now. 👍 here |
I really tried to simplify that but the go formatter/parsers just weren't having it. I wish there were a "See this AST I just modified? Fix the positions for me.". |
(10x)
Question: why did we use transitive dependencies when computing rewrites? IMO, we should definitely not be doing that. It's also slow.