Make CodeMap and FileMap thread-safe

[Zoxc]

r? @michaelwoerister

[michaelwoerister]

Thanks for the PR, @Zoxc! The code looks correct to me. What we really should do though is make FileMap immutable. Except for the external_src business, a FileMap should only ever be modified during parsing. Maybe we can refactor things so that the parser works with a &mut FileMap?

[michaelwoerister]

I think it would make sense to put stable_id_to_filemap into the same Lock as files since they should be modified atomically.

[Zoxc]

We could probably also remove files altogether since stable_id_to_filemap seems to contain the same information?

[michaelwoerister]

We address FileMaps by their index in the files array quite a bit. For now, I would just keep both data structures.

[michaelwoerister]

lines, multibyte_chars, and non_narrow_chars should probably be within the same Lock too.

[Zoxc]

These fields aren't atomically updated, but incrementally updated during parsing. So they aren't really thread-safe until after parsing. We should probably split them out of FileMap and atomically update them after parsing, or pass &mut FileMap to the parser, so no one can observe them changing. We could also use compute them lazily with a query. That may be a performance win if they are unused when no diagnostics are emitted.

[michaelwoerister]

We could also use compute them lazily with a query. That may be a performance win if they are unused when no diagnostics are emitted.

I think they are used almost always because we need them for debuginfo and panic messages.

I would like FileMap to be immutable if possible. The parser could keep it's own mutable version and only register it with the CodeMap when it is finished (at which point the codemap could apply the relevant offsets), or the parser could reserve the address range beforehand if that's much more performant.

[michaelwoerister]

@Zoxc, will you be looking into making FileMap immutable as part of this PR or should we do it in a subsequent one?

[Zoxc]

I'd prefer to do that in a separate PR. I added it to my list of things to fix.

[michaelwoerister]

OK. The issue with stable_id_to_filemap and files being in separate mutexes should be solved in this PR though. Can you look into it? Otherwise this is good to go, I think!

[Zoxc]

I added a commit which addresses that.

[michaelwoerister]

@bors r+

[bors]

📌 Commit 65b4990 has been approved by michaelwoerister

[bors]

⌛ Testing commit 65b4990 with merge c3fd5d0...

[bors]

☀️ Test successful - status-appveyor, status-travis
Approved by: michaelwoerister
Pushing c3fd5d0 to master...