Add support for immutable xvm structures

[cpurdy]

This is the next chunk of work to help support bundles, efficient caching, etc. It's not a fix to anything in and of itself, but a building block that we need. As an aside, there are going to now be opportunities to efficiently replace things like array based APIs with e.g. List, now that we have mutable vs immutable in the structures.

[lagergren]

This is a building block I want, and some of it is more careful than it looks — verifyMutable() is called as a statement rather than inside an assert, so it survives -da, and Contribution.markReadOnly() copies the collections, which quietly neutralises any reference a caller took while the structure was still mutable. I nearly filed that second one as a leak before working out why it's safe.

One question decides most of my comments, and I'd rather ask than assume: can a structure be frozen after it has been shared, or is the rule "build, freeze, then publish"?

If it's build-freeze-publish, then m_fReadOnly as a plain boolean is right and cheaper than volatile (even though the overhead is negligible) — publication supplies the happens-before edge for the flag and everything reachable from it. I'd only want that contract written down, because nothing states it today and the API makes the other order equally easy to write: ensureReadOnly() acts on a live object, markReadOnly() is itself a mutation, and ensureMutable()/clone() mint a mutable structure from a frozen one at an arbitrary later time.

Worth flagging either way: #549 is open and argues MethodStructure.m_fNative and m_code must be volatile because they're "written by one thread and read by others". MethodStructure is one of the classes this PR adds m_fReadOnly to. Those two PRs should agree before either lands — and if #549's premise is right, this one's answer follows.

The larger thing, not blocking: this makes immutability advisory — a runtime check every current and future mutator has to remember, with no way to say "I require an immutable structure" in a signature. That's fine as a first step, but it's worth deciding now whether the destination is structural (a distinct immutable type, or freeze-by-construction returning a new instance with final fields), because that changes what's worth building on top of this. I would DEFINITELY like something that has an implicit before and after edge like finals rather than volatiles or "these mutable fields are definitely not going to be in a racy situation ever."

[lagergren]

Line-level notes to go with my summary comment above. The first one is the only one I'd call blocking; the rest are readability and contract questions.

[cpurdy]

One question decides most of my comments, and I'd rather ask than assume: can a structure be frozen after it has been shared, or is the rule "build, freeze, then publish"?

Ideally, when we load from disk (or from byte[] or whatever), we will either get a read-only form, or we will need to say "instantiate this thing in a mutable form" or "instantiate this thing in a read-only form".

When we create one from scratch (e.g. a compiler), it is a mutable form. Obviously.

From a mutable form, it can be transitioned to a read-only form. From a read-only form, it must be copied to obtain a mutable form.

When mutable, the data model is not intended to be (nor is in any way) thread safe. Trying to use it in a multi-threaded manner when it is mutable is a fool's errand. Don't do that.

When read-only, the data model should be thread safe. I cannot (yet) warrant that it is, but it should be reasonably close outside of tests explicitly intended to show otherwise.

Again, the intention is simple: single threaded if mutable, and only after explicitly made immutable should any thread even get to look at the structure.

If it's build-freeze-publish, then m_fReadOnly as a plain boolean is right and cheaper than volatile (even though the overhead is negligible) — publication supplies the happens-before edge for the flag and everything reachable from it.

Something like this. I'm trying to retrofit this on top of an "organically evolved" data structure. It's not going to immediately come out as perfect.

markReadOnly() is itself a mutation

Sure. But note that it does nothing to something that is already read-only. So the model is consistent.

Worth flagging either way: #549 is open and argues MethodStructure.m_fNative and m_code must be volatile because they're "written by one thread and read by others".

Actually, that issue is one of the reasons that I did this project and did it now. The idea of "written by one thread and read by others" is obviously incompatible with what I've described above.

The larger thing, not blocking: this makes immutability advisory — a runtime check every current and future mutator has to remember, with no way to say "I require an immutable structure" in a signature.

Correct. This is Java, so we're largely out of luck in this regard anyhow.

The point is: As we utilize the read-only option, e.g. by default on loading, tools that need a mutable form will need to explicitly ask for a mutable form, or they'll blow up. I'm ok with that as a short term problem to fix. And when we do fix it, it should eliminate some of the bleed-through issues that either already do happen or that we fear could happen.

[lagergren]

One question decides most of my comments, and I'd rather ask than assume: can a structure be frozen after it has been shared, or is the rule "build, freeze, then publish"?

Ideally, when we load from disk (or from byte[] or whatever), we will either get a read-only form, or we will need to say "instantiate this thing in a mutable form" or "instantiate this thing in a read-only form".

When we create one from scratch (e.g. a compiler), it is a mutable form. Obviously.

From a mutable form, it can be transitioned to a read-only form. From a read-only form, it must be copied to obtain a mutable form.

When mutable, the data model is not intended to be (nor is in any way) thread safe. Trying to use it in a multi-threaded manner when it is mutable is a fool's errand. Don't do that.

When read-only, the data model should be thread safe. I cannot (yet) warrant that it is, but it should be reasonably close outside of tests explicitly intended to show otherwise.

Again, the intention is simple: single threaded if mutable, and only after explicitly made immutable should any thread even get to look at the structure.

If it's build-freeze-publish, then m_fReadOnly as a plain boolean is right and cheaper than volatile (even though the overhead is negligible) — publication supplies the happens-before edge for the flag and everything reachable from it.

Something like this. I'm trying to retrofit this on top of an "organically evolved" data structure. It's not going to immediately come out as perfect.

markReadOnly() is itself a mutation

Sure. But note that it does nothing to something that is already read-only. So the model is consistent.

Worth flagging either way: #549 is open and argues MethodStructure.m_fNative and m_code must be volatile because they're "written by one thread and read by others".

Actually, that issue is one of the reasons that I did this project and did it now. The idea of "written by one thread and read by others" is obviously incompatible with what I've described above.

The larger thing, not blocking: this makes immutability advisory — a runtime check every current and future mutator has to remember, with no way to say "I require an immutable structure" in a signature.

Correct. This is Java, so we're largely out of luck in this regard anyhow.

The point is: As we utilize the read-only option, e.g. by default on loading, tools that need a mutable form will need to explicitly ask for a mutable form, or they'll blow up. I'm ok with that as a short term problem to fix. And when we do fix it, it should eliminate some of the bleed-through issues that either already do happen or that we fear could happen.

This is the contract I wanted, and I guess I am just looking for ways that the code clearly enforces it, so that I cannot accidentally not do this. But as I've said all the time - this is fine in the current environment and merge it as is. Would be cool to show you how a "replaced representation" example will "work" now e.g. Lists instead of Arrays in ways it didn't "work" before. And some unit tests that exercise the API would be nice mainly as documentation.

[lagergren]

One question decides most of my comments, and I'd rather ask than assume: can a structure be frozen after it has been shared, or is the rule "build, freeze, then publish"?

Ideally, when we load from disk (or from byte[] or whatever), we will either get a read-only form, or we will need to say "instantiate this thing in a mutable form" or "instantiate this thing in a read-only form".
When we create one from scratch (e.g. a compiler), it is a mutable form. Obviously.
From a mutable form, it can be transitioned to a read-only form. From a read-only form, it must be copied to obtain a mutable form.
When mutable, the data model is not intended to be (nor is in any way) thread safe. Trying to use it in a multi-threaded manner when it is mutable is a fool's errand. Don't do that.
When read-only, the data model should be thread safe. I cannot (yet) warrant that it is, but it should be reasonably close outside of tests explicitly intended to show otherwise.
Again, the intention is simple: single threaded if mutable, and only after explicitly made immutable should any thread even get to look at the structure.

If it's build-freeze-publish, then m_fReadOnly as a plain boolean is right and cheaper than volatile (even though the overhead is negligible) — publication supplies the happens-before edge for the flag and everything reachable from it.

Something like this. I'm trying to retrofit this on top of an "organically evolved" data structure. It's not going to immediately come out as perfect.

markReadOnly() is itself a mutation

Sure. But note that it does nothing to something that is already read-only. So the model is consistent.

Worth flagging either way: #549 is open and argues MethodStructure.m_fNative and m_code must be volatile because they're "written by one thread and read by others".

Actually, that issue is one of the reasons that I did this project and did it now. The idea of "written by one thread and read by others" is obviously incompatible with what I've described above.

The larger thing, not blocking: this makes immutability advisory — a runtime check every current and future mutator has to remember, with no way to say "I require an immutable structure" in a signature.

Correct. This is Java, so we're largely out of luck in this regard anyhow.
The point is: As we utilize the read-only option, e.g. by default on loading, tools that need a mutable form will need to explicitly ask for a mutable form, or they'll blow up. I'm ok with that as a short term problem to fix. And when we do fix it, it should eliminate some of the bleed-through issues that either already do happen or that we fear could happen.

This is the contract I wanted, and I guess I am just looking for ways that the code clearly enforces it, so that I cannot accidentally not do this. But as I've said all the time - this is fine in the current environment and merge it as is. Would be cool to show you how a "replaced representation" example will "work" now e.g. Lists instead of Arrays in ways it didn't "work" before. And some unit tests that exercise the API would be nice mainly as documentation.

BTW - There are ways to do som of this in Java. I would define a trivial wrapper FrozenArray, which will, in all situations be absolutely as performant as a raw array, but with none of the mutability and Object:s.