refactor!: Restore full copy-on-write tree snapshots, now using immutable-chunkmap

[mwcampbell]

To avoid semver-breaking version increases on the adapter crates, I think we'll have to split this into two PRs: one for the changes to accesskit_consumer, and the other for the changes to the adapters. This will require us to merge the first PR with broken CI, and briefly break the main branch. But I can't think of a better way.

[DataTriny]

Looks good to me! I don't see another way to avoid a breaking version increase either.

The CI passes here, so you can go ahead and extract changes to the adapters.

[mwcampbell]

Looking closer at the immutable-chunkmap project, I'm concerned about depending on it. First, it adds several indirect dependencies, including the heavy proc-macro-related dependencies (syn and quote), across all platforms. All of the transitive dependencies of immutable-chunkmap, except for arrayvec (which is ubiquitous across the Rust ecosystem), are for the sake of conveniently defining a packed struct (HeightAndSize in src/avl.rs). If it's even really worthwhile for that struct to be packed, it would be easy enough to hand-write packing and unpacking in safe Rust.

Speaking of safety, the README says the project is written entirely in safe Rust, yet there are several instances of unsafe, again in src/avl.rs. The library used to forbid unsafe code, but I don't see any explanation about why that practice was dropped.

And in terms of process, the project doesn't have CI (that I can see) or tagged releases.

[DataTriny]

Thanks for digging deeper into this. Not having tagged releases is unacceptable.

All of the issues you listed can probably be solved fairly easily, assuming the maintainer is willing to accept PRs. But depending on this crate right now is probably not a good idea...

This would bump into our short-term goals though...

[mwcampbell]

Well, we could fork immutable-chunkmap. I have already been considering that option. The easiest way to eliminate the unsafe code, after all, is to simply drop the few functions that require it; these are the mutable iterator, get_or_insert_cow, and get_or_default_cow, none of which we need. As for the packed struct, which is the source of most of the dependencies, my preference is to eliminate the packing altogether. But the original author may disagree about the value of minimizing dependency count versus winning at benchmarks; given the size of the bench directory, they seem to care a lot about the latter.

AccessKit would be the first popular public project depending on immutable-chunkmap, so we wouldn't be causing a significant division in the Rust ecosystem by forking.

What do you think?

[DataTriny]

Let's wait and see how the author reacts to the PRs you've open.

To be exhaustive, I should mention that the crates isn't no_std compatible, but a quick look at its code seem to indicate that it would be feasible to implement. This is not a priority for us at the moment, but we should have it in our minds nonetheless.

[mwcampbell]

I'm going to bring this up to date. Then I'd like to go ahead and merge it, even though immutable-chunkmap still depends on packed_struct. I've just submitted a PR to immutable-chunkmap with my own implementation of packing the height and size. If Eric accepts that, as he said he'd be open to doing if it passes review, then that will eliminate all of the transitive dependencies of immutable-chunkmap except for arrayvec, which is widespread. Anyway, this PR, and the work on AT-SPI text support, have been stuck long enough.

[mwcampbell]

I'm done bringing this PR up to date. Maybe it could use another review now. Also, we were planning to actually merge this as two PRs, a refactor! for accesskit_consumer since we're making a breaking change to that crate, and a fix for the other crates to avoid a semver-breaking version bump for those.

[DataTriny]

In consumer/src/tree.rs you use ChunkMap::get to check for the presence or absence of a node. You could use get_key instead, which might have slightly less work to do?

Otherwise this looks good to me. So please split this PR in two.

[mwcampbell]

OK, now this branch only has the changes to the consumer crate. Of course, this breaks CI. But we'll fix that in the next PR.