laravel-achievements
laravel-achievements copied to clipboard
Inefficient use of database
I saw that achievement generation is quite inefficient when it comes to database usage.
The culprit is getModel
method that is called 3 times for each achievement progress modification:
- in constructor
- in
getOrCreateProgressForAchiever
to fetch achievement id - in
getOrCreateProgressForAchiever
to associate newly created progress with details model
Then in this getModel
method we have 2 database calls: read
and save
That makes 6 database calls (8 if there is an update) for each achievement update.
My suggestion would be to make a dedicated command for synchronizing achievement details with database and remove save
call from getModel
as well as getModel
call from constructor (possibly as a config option for backword compatibility) .
An extra improvement would be to reuse results of getModel
instead of calling it each time.
Another suggestion would be to add an option for having stable ids (eg. calculated from md5 of achievement fully qualified class name or just provided by user) and assume that those will match model's id, but that is something to be discussed and since its a super breaking change, definitely require a config option.
Hi @hubertnnn ! Thanks a lot for the feedback. I 100% agree that the current base is not as efficient as it could be, and will be taking a look at this for the next release (scheduled for January).
If you would like to help, please submit a pull request!
Hi @hubertnnn !
The latest version on master implements your suggested changes.
https://github.com/gstt/laravel-achievements/commit/1faf09efccfc08253e7f2f364a57dd896eea1034
Please let me know if this change works for you!