search

amazon-ion / ion-schema-rust

Rust implementation of Ion Schema
https://amazon-ion.github.io/ion-schema/sandbox
Apache License 2.0
12 stars 6 forks source link

adds changes for making `SchemaSystem` Send and Sync #153

Closed desaikd closed 1 year ago

desaikd commented 1 year ago

Issue #151, #150

Description of changes:

This PR works on making SchemaSystem Send and Sync.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

desaikd commented 1 year ago

While working on a test for multi-thread use of schema system I realized. As ion-schema-rust still uses ion-rs v0.15.0 which uses Rc<Vec<Element>> (See https://github.com/amazon-ion/ion-rust/blob/v0.15.0/src/value/owned.rs#L147). This also made me realize that in order for SchemaSystem to be fully Send ans Sync the Element API also has to be Send and Sync. So even though I might have been able to make load_schema return Arc<Schema>, it doesn't necessarily make SchemaSystem Send and Sync.

popematt commented 1 year ago

While working on a test for multi-thread use of schema system I realized. As ion-schema-rust still uses ion-rs v0.15.0 which uses Rc<Vec>

Do you want to put this PR on hold until after we've upgraded to ion-rs = "0.16.0" or merge this PR and then do the ion-rs upgrade?

desaikd commented 1 year ago

While working on a test for multi-thread use of schema system I realized. As ion-schema-rust still uses ion-rs v0.15.0 which uses Rc

Do you want to put this PR on hold until after we've upgraded to ion-rs = "0.16.0" or merge this PR and then do the ion-rs upgrade?

I am thinking to hold this PR. I have already worked on upgrade to v0.16.0 and wil shortly create a PR for that. Although we need to wait for v0.17.0 until we can resolve this because ion-rs v0.16.0 uses Rc for SymbolText enum (See https://github.com/amazon-ion/ion-rust/blob/v0.16.0/src/symbol.rs#L13). This has already been changed to use Arc but not yet released. (See https://github.com/amazon-ion/ion-rust/pull/497)

Holding this until we update ion-rs to v0.17.0.