laravel-achievements icon indicating copy to clipboard operation
laravel-achievements copied to clipboard

Inefficient use of database

Open hubertnnn opened this issue 5 years ago • 2 comments

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.

hubertnnn avatar Nov 29 '18 19:11 hubertnnn

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!

gabs-simon avatar Nov 30 '18 16:11 gabs-simon

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!

gabs-simon avatar Dec 31 '18 23:12 gabs-simon