CI is failing. :(

Will review, but first, a question.

Because of vendoring, there is no strict mapping between import path -> PackageData -> Archive any more.

Can you elaborate on what you mean?

My understanding of /vendor/ was previously mistaken, and I would've said the same thing. But by now I think there's a very direct mapping between import path (original with /vendor/ prefix kept) and build packages.

What I mean is that there are two different versions of the import path now:

1. "local" import path written in the import statement
2. "real" import path of the imported package
   A compiler needs to be very careful to not confuse the two. E.g. the same local import path can point to different packages, so having a cache "local import path -> package archive" is bad.

Got it, that makes sense.

On a side note, we need to come up with good terminology to differentiate those 2 types of import paths. :/

There's now a relative import path (e.g., "./foo/bar"), a full import path ("example.com/user/repo/foo/bar"), and a... even more full import path ("example.com/user/otherrepo/baz/vendor/example.com/user/repo/foo/bar")...

Reviewed, see comments/questions above.

This is definitely an improvement in data organization. Although a lot of this code can go away once we're using cmd/go. ;)

LGTM, but read my comments.

Two things after merging this (I will do):

1. Keep an eye on compilation times before and after this change, since it touches code that may affect that.
2. Check if https://github.com/shurcooL/gopherjslib needs to be updated.

@shurcooL Thx!

There seems to be a regression with compilation times. Compiling https://github.com/shurcooL/eX0/tree/master/eX0-go#readme with previous version:

eX0-go $ time gopherjs build --tags=tcp

real    0m0.614s
user    0m0.513s
sys 0m0.214s
eX0-go $ time gopherjs build --tags=tcp

real    0m0.649s
user    0m0.538s
sys 0m0.231s
eX0-go $ time gopherjs build --tags=tcp

real    0m0.603s
user    0m0.520s
sys 0m0.210s

0.6 seconds on average.

After updating:

eX0-go $ time gopherjs build --tags=tcp

real    0m11.203s
user    0m14.055s
sys 0m2.418s
eX0-go $ time gopherjs build --tags=tcp

real    0m1.397s
user    0m1.383s
sys 0m0.440s
eX0-go $ time gopherjs build --tags=tcp

real    0m1.363s
user    0m1.345s
sys 0m0.437s
eX0-go $ time gopherjs build --tags=tcp

real    0m1.395s
user    0m1.384s
sys 0m0.443s

First time it takes longer because all dependencies need to be rebuilt since gopherjs binary has changed. But subsequent times are still 2x as long as before (1.3 second).

I've noticed a similar significant compilation time increase in gopherjslib and I thought it might be specific to its code, but it seems to affect gopherjs too.

The Go package above doesn't use /vendor/ anywhere. Any ideas why it'd get slower?

Edit: It may use /vendor/ by importing net/http which vendors http2 inside...

This is probably because it is calling https://golang.org/pkg/go/build/#Context.Import much more often. This is necessary because this function resolves vendored packages. We may add caching so we only do it once for every dependency of a package instead of for each import statement in each file of the package.

Makes sense. Shall we open an issue to track that?

An alternative to consider is reverting this change, to keep working on it on a branch until we're happy with performance, and merge it then.

We can use the Go profiler to profile compilation time and more easily find out where the bottlenecks are.

Please test if #413 improves the situation.

Revert: No, because this fixed bugs. My initial vendoring implementation was not correct. I'd rather have it slower than broken.

Agreed.

#413 makes a significant improvement, but it's not 1:1 with original performance. It's definitely good enough to proceed with this and consider optimizing later on.

goReadersToJs taken: 3.971618929s - before 411
goReadersToJs taken: 12.916400449s - after 411
goReadersToJs taken: 5.217832774s - after 413

Cool, I'll merge it.