Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[ECO-5013] feat: Implement room lifecycle monitoring #94

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

sacOO7
Copy link
Contributor

@sacOO7 sacOO7 commented Jan 6, 2025

  1. Implemented thread-safe ScopeEmitter ( Behaviour might look same as Hot Flows, but it avoid problems associated with hot flows. Added comment -> [ECO-5013] feat: Implement room lifecycle monitoring #94 (comment)
  2. Added impl. for the same as a part of RoomLifeCycleManager

@github-actions github-actions bot temporarily deployed to staging/pull/94/dokka January 6, 2025 12:52 Inactive
1. Defined standard Emitter interface as per RTE emitter spec
2. Added thread safe impl. for the same in ScopedEmitter class
3. Added private AsyncSubscriber class to process events in the given order
2. Added related unit tests covering all edge cases
@sacOO7 sacOO7 force-pushed the feature/CHA-RL4-room-monitoring branch from 4c60f8b to a7323c1 Compare January 7, 2025 11:59
@github-actions github-actions bot temporarily deployed to staging/pull/94/dokka January 7, 2025 12:01 Inactive
Copy link

github-actions bot commented Jan 7, 2025

Code Coverage

File Coverage [91.39%]
chat-android/src/main/java/com/ably/chat/Emitter.kt 95.35%
chat-android/src/main/java/com/ably/chat/RoomLifecycleManager.kt 90.82%
Total Project Coverage 86.30%

* Kotlin Emitter interface for supplied value
* Spec: RTE1
*/
internal interface Emitter<V> {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don’t think we need this, as we’re essentially reinventing Flow here. Additionally, we’re missing some crucial points in the ScopedEmitter implementation. In its current form, listeners will be invoked non-sequentially, which can be misleading.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was hoping we could have a discussion about this eventually. I don't think we're trying to reinvent the wheel here, though the implementation might resemble shared or hot flows.

Unlike cold flows, hot flows continue emitting values even if no one is actively collecting them. This can lead to situations where the flow keeps emitting values even when the component is in the background, which could potentially cause memory leaks.

Additionally, the flow API is well-suited for use in coroutine-based functions, as both emit and collect are suspending functions. On the other hand, tryEmit can be used in non-suspending functions (like the channel.on method here), although it's generally not recommended.

Since tryEmit() is not a suspending function, it requires a buffer to store the emitted values until they are processed by subscribers. To make this work, we need to set the buffer size to unlimited, but this can affect performance and slow down other subscribers.

Another challenge is canceling flows, as there’s no guarantee that queued events will be delivered when cancellation occurs.

I'll create a separate thread to dive deeper into this and explore how the Emitter implementation can be leveraged for the existing public API to handle messages and presence events asynchronously on the Dispatchers.Default scope, as opposed to relying on the current blocking subscribers.

1. Updated init method to include setting up ContributorListeners
2. Initialized property contributorStateChangeMonitor using ScopedEmitter
to track feature channel state changes
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants