Asynchronous Inbox + asynchronous house saving #4561
Conversation
… single std::recursive_mutex, TODO: Deliver items using IOInbox::pushDeliveryItems when Inbox is not yet loaded for player
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
ghost
requested review from
There was a problem hiding this comment.
I liked the PR, it looks promising.
I made some improvements, removed duplicate code, see
https://github.com/ramon-bernardo/forgottenserver/pull/2
I'm still reviewing IOInbox!
src/player.cpp
Outdated
| { | ||
| inbox = _inbox; | ||
| if (depotLocker) { | ||
| depotLocker->internalAddThing(2, inbox); |
There was a problem hiding this comment.
the index here could be a const?
ghost
added
the
…ng inbox, the delivery should now be guaranteed and IOInbox::flush is now only necessary when g_dispatcherInbox does not accept new tasks, needs further research if IOInbox::flush() can be removed
gunzino
changed the title
gunzino
changed the title
|
@ramon-bernardo |
gunzino
changed the title
| { | ||
| static Database asyncInstance; | ||
| return asyncInstance; | ||
| } |
There was a problem hiding this comment.
IIRC getInstance already returns an "async" instance, used for other purposes but should be suitable. Why having another instance?
On that note, singleton sucks, but a "doubleton" is even worse 😆 let's just make it not a global and pass it around
There was a problem hiding this comment.
What do you mean byt getInstance returns "async" instance? Is Database class thread-safe? Can database class execute multiple parallel queries from several threads? The asyncInstance is there only to be used by g_asyncTasks thread for async database IO- in this PR it is now only House and Inbox.
| std::map<uint32_t, PlayerDBEntryPtr> inboxCache; | ||
| std::map<uint32_t, std::list<ItemBlockList>> pendingItemsToSave; | ||
| std::set<uint32_t> pendingPlayerSet; | ||
| }; |
There was a problem hiding this comment.
Make it a namespace and move everything private to the .cpp file only as this is not used anywhere else
There was a problem hiding this comment.
Is there similar approach somewhere else in the code? What about recursive mutex?
|
This looks like a very promising PR, what is the status of this? |
Changes Proposed
I'm happy to share a contribution to the TFS which consists of async, zero-blocking Inbox implementation to prevent abusing the current sync system with large Inboxes and causing lags due to database IO like mentioned here #1431. The similar implementation is rotating between server owners but does not include complete zero-blocking IO for both saving and loading and parcel/item delivery to Inbox when player is offline. The implementation can be further used for more components like depots, store inbox etc. but those are currently either limited or controlled by server. The basic concept was created by kondra ([email protected]) few years ago but the code is already widespread across server owners which some of them resell for real money.
The implementation highlights:
Items delivery to inbox currently done for:
EDIT (21th November): Added asynchronous houses saving
I would be really happy for your hints/recommendations regarding so this contribution can be pushed to the master branch.