Rust::com General Optimization and cleanup of RUST-API#276
Rust::com General Optimization and cleanup of RUST-API#276bharatGoswami8 wants to merge 6 commits intoeclipse-score:mainfrom
Conversation
bharatGoswami8
changed the title
bharatGoswami8
force-pushed
the
com_api_optimization
branch
3 times, most recently
from
a8502a3 to
9219844
Compare
bharatGoswami8
requested review from
LittleHuba,
bemerybmw,
castler,
crimson11,
hoe-jo and
limdor
as code owners
bharatGoswami8
force-pushed
the
com_api_optimization
branch
3 times, most recently
from
7bd3067 to
6fb410c
Compare
| .store(true, std::sync::atomic::Ordering::Relaxed); | ||
| } | ||
| // We are using std::sync::Once to ensure that the callback is set only once. | ||
| self.async_init_status.call_once(|| { |
There was a problem hiding this comment.
std::sync::Once::call_once is a blocking, thread-parking primitive. Calling it inside an async block violates the fundamental async contract: futures must never block the executor thread.
suggestion: OnceLock (lock-free, non-blocking) or keep previous AtomicBool
There was a problem hiding this comment.
Async receive enforces single-threaded access via ProxyEventManager's in_progress acquire/release lock, concurrent calls panic immediately. call_once executes only once on first receive() call, blocking for microseconds (FFI callback setup only) because init should be done before returning future.
And also sync::once give clear indication to developer this is one time call.
| @@ -259,23 +258,18 @@ impl NativeProxyEventBase { | |||
| pub fn new(proxy: *mut ProxyBase, interface_id: &str, identifier: &str) -> Result<Self> { | |||
There was a problem hiding this comment.
Yes why not, we can take as &NonNull
| ProducerFailedReason::SkeletonCreationFailed, | ||
| )); | ||
| } | ||
| let skeleton_event_ptr = std::ptr::NonNull::new(raw_event_ptr).ok_or( |
There was a problem hiding this comment.
line 310 used ok_or_else, here ok_or, seems inconsistent. please check other places as well.
There was a problem hiding this comment.
Yes it was only in this line updated.
| )); | ||
| } | ||
| let skeleton_event_ptr = std::ptr::NonNull::new(raw_event_ptr).ok_or( | ||
| Error::ProducerError(ProducerFailedReason::SkeletonCreationFailed), |
There was a problem hiding this comment.
SkeletonCreationFailed? is this correct?
| /// As it has Send and Sync unsafe impls, it must not expose any mutable access to the skeleton handle | ||
| pub struct NativeSkeletonHandle { | ||
| pub handle: *mut SkeletonBase, | ||
| pub handle: NonNull<SkeletonBase>, |
There was a problem hiding this comment.
pub? comment say it must not expose any mutable access to the skeleton handle
There was a problem hiding this comment.
Thank you for pointing out this. Looks like in producer side few places still pub keyword was there for field.
* Changed init flag from automic to std::once
* Changed NativeSkeletonHandle to NonNull
* Updated Naming for Lola and Mock runtime internal type
* Consumer proxy and event type field updated to NonNull
* Updated receive algo document
bharatGoswami8
force-pushed
the
com_api_optimization
branch
from
6fb410c to
247b5d1
Compare
* Address the review comment
bharatGoswami8
force-pushed
the
com_api_optimization
branch
from
247b5d1 to
c49490e
Compare
3 participants
Uh oh!
There was an error while loading. Please reload this page.