Adopt ring buf for queued notifications #188
Conversation
github-actions
bot
added
comp/santad
mlw
force-pushed
the
notify-queue-ring-buf
branch
from
a8b2f10 to
347720f
Compare
|
|
||
| /// For each pending notification, call the reply block if set then clear the | ||
| /// reply so it won't be called again when the notification is eventually sent. | ||
| - (void)clearAllPendingRepliesLocked { |
There was a problem hiding this comment.
The "locked" in this method name (and flushQueueLocked) is a little confusing as there isn't a lock held
There was a problem hiding this comment.
We have a few of those in the code base, by convention they assume that you've already locked the resource.
There was a problem hiding this comment.
Discussed offilne: none of these cases with the 'Locked' suffix are using locks explicitly but are only being done on serialized queues, so safety-wise it's OK. Switching the naming to be clearer might be beneficial but is not pressing.
|
|
||
| /// For each pending notification, call the reply block if set then clear the | ||
| /// reply so it won't be called again when the notification is eventually sent. | ||
| - (void)clearAllPendingRepliesLocked { |
There was a problem hiding this comment.
Discussed offilne: none of these cases with the 'Locked' suffix are using locks explicitly but are only being done on serialized queues, so safety-wise it's OK. Switching the naming to be clearer might be beneficial but is not pressing.
This change makes the SNTNotificationQueue use a ring buffer for storing queued messages instead of an array. This means that the most recent notifications will be displayed when a user logs in and the oldest ones will be dropped.