urlyy opened a new pull request, #2737:
URL: https://github.com/apache/fory/pull/2737

   ## What does this PR do?
   
   1. Remove the lifetime annotations from `WriteContext` and `ReadContext` 
because managing their lifetimes was too troublesome. The **trade-off** is 
that,  previously, `MetaWriterResolver` held references to the corresponding 
`TypeDef` obtained from `TypeResolver`, and both contexts also fetched  
references to `Harness` objects managed by `TypeResolver`. Now, all these 
references have been replaced with `clone()` to simplify lifetime management. 
This may introduce a non-negligible performance overhead, but it was the only 
practical solution I could implement.🥲
   
   2. Remove `&fory` from the member variables of `Context`. Instead, `&fory` 
is now passed in as a function parameter rather than being retrieved via 
`context.get_fory()`. This change modified many API signatures.
   
   3. Add two pools to `Fory`’s member variables to allow reuse of both types 
of contexts. Tests confirmed that the same context addresses were reused 
multiple times. However, since automatic recycling would require `Arc<Fory>`, 
only manual recycling has been implemented so far — this operation is handled 
internally(within `serialize()/deserialize()`), and users don’t need to recycle 
contexts manually. Also, for the `de/serialize_with_context(context)` 
functions, if users call them directly, the user's manually managed contexts 
will **not** be returned to the pool.
   
   4. Add `reset()` methods to be executed on `Context objects` before 
recycling. You may consider whether to unify the naming convention to either 
`clear` or `reset`.
   
   5. Change the type of `Fory::metastring_resolver` from 
`Rc<RefCell<MetaStringResolver>>` to `Arc<Mutex<MetaStringResolver>>`, modified 
`TypeResolver::sorted_field_names_map` from `RefCell<...>` to `RwLock<...>`, 
and changed `MetaReaderResolver::reading_type_defs` from `Vec<Rc<TypeMeta>>` to 
`Vec<Arc<TypeMeta>>`. In addition, `RefReader` was unsafely marked as `Send` 
and `Sync`. These changes allow `Fory` to support serialization across multiple 
threads. A concrete example can be found in 
`rust/tests/tests/test_multi_thread.rs`. However, I’m not sure whether using 
`Lock` and `Arc` will impact single-thread performance.
   
   6. The reason why I `unsafe impl Send/Sync for RefReader {}`, is that , 
`RefReader` contains `Box<dyn Any>`, which by default is not `Send`/`Sync`. But 
In our usage, each ref_reader is only accessed within a context, by `one` 
thread at a time, so it is safe to implement `Send`/`Sync` manually using 
`unsafe`.
   
   
   ## Related issues
   - close #2717 


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to