igrigorik / vimgolf

Real Vim ninjas count every keystroke - do you?
http://www.vimgolf.com/
MIT License
679 stars 65 forks source link

Paginate solutions - reduce memory consumption #248

Closed Hettomei closed 6 years ago

Hettomei commented 6 years ago

You may not love the 'style' since I do not use Rails magic to create/use object. But I cannot find ways to 'not load all entries' when using rails models

What have been done :

Additionnal infos :

About 'reduce memory consumption' I use :

pp NewRelic::Agent::Samplers::MemorySampler.new.sampler.get_sample

At the beginning and at the end of 'ChallengeController#show' It looks like it seems close to what heroku returns.

with

Before this PR : beginning of #show 136, end of #show 191.578125 -> I get lot of requestTimeout, and MemorySampler varie from 110 to 210

With this PR : beginning of #show 136, end of #show 136 -> no requestTimeout, and MemorySampler is stable

igrigorik commented 6 years ago

@Hettomei this is a big one, kudos for taking it on. Left a few nitpick suggestions above.

What's left, modulo fixing the failing build?

Hettomei commented 6 years ago

What's left

Hettomei commented 6 years ago

Implement the system that display solutions step by step (based on the score of players)

--> done

What's left

igrigorik commented 6 years ago

@Hettomei any progress on your end? Would love to land this :-)

Hettomei commented 6 years ago

Hi thanks for the reminder.

I have a working solution on another branch (paginate-solution-2), but I don't like it:

I have an idea to simplify the number/difference/maintenability of queries but I need to take time for that. And I will have time at the end of july

igrigorik commented 6 years ago

@Hettomei 👍

Hettomei commented 6 years ago

updated description : only test left. Still WIP

Hettomei commented 6 years ago

I updated the title : It is no more a WIP, its ready to merge :tada:

@igrigorik can you review it ?

I tested with the classic big challenge http://www.vimgolf.com/challenges/55b18bbea9c2c30d04000001 -> for my twitter account, the page display the same information with this PR.

igrigorik commented 6 years ago

@Hettomei nice work! Have you done any perf testing? Curious what the relative gains are here.

Hettomei commented 6 years ago

About perf : In prod env, after 'warming up' the page I will push what rails gives (ms to display a view) I will push memory used.

I don't know how to do other good benchmark.

About the review : thanks, i'll update asap.

Hettomei commented 6 years ago

Benchmark

On a Virtual machine with debian. (2GO ram)

"display view" use rails built in output "ram" use pp NewRelic::Agent::Samplers::MemorySampler.new.sampler.get_sample At the end of 'ChallengeController#show' When performing 'curl' with 'best user' I update app/controllers/application_controller.rb to force the connected usr with the best

Given :

In dev env

With current master :

disconnected player

(only see lastest responses)

display view: I, [2018-07-24T23:12:59.876847 #3573] INFO -- : Completed 200 OK in 3078ms (Views: 2328.8ms)

ram: fluctuate between 250 and 256 Seems to be garbage collected

time curl: real 0m3.313s

Best player

(see all response)

display view: I, [2018-07-24T23:08:56.726616 #2888] INFO -- : Completed 200 OK in 7287ms (Views: 6570.7ms)

ram: fluctuate between 272 and 286 Seems to be garbage collected every request

time curl: real 0m7.396s

I sometimes get E, [2018-07-24T23:11:06.227065 #2348] ERROR -- : worker=0 PID:2888 timeout (16s > 15s), killing and see 'Rack timeout error' in browser

With current pull request :

disconnected player

(only see lastest responses)

display view: I, [2018-07-24T22:13:23.903541 #10426] INFO -- : Completed 200 OK in 1077ms (Views: 1067.8ms)

ram: start at "134.62" then grow 0.26 more every 5 requests

time curl: real 0m1.100s

Best player

(see all response)

display view: I, [2018-07-24T22:28:09.391641 #23661] INFO -- : Completed 200 OK in 1192ms (Views: 1187.3ms)

ram: start at "134.62" then grow 0.26 more every 5 requests

time curl: real 0m1.170s

prod env

With current master :

disconnected player

(only see lastest responses)

display view: I, [2018-07-24T22:56:38.198144 #31907] INFO -- : Completed 200 OK in 3139ms (Views: 2213.9ms)

ram: fluctuate between 220 and 238 Seems to be garbage collected every request

time curl: real 0m2.986s

Best player

(see all response)

display view: I, [2018-07-24T23:00:06.377434 #503] INFO -- : Completed 200 OK in 6682ms (Views: 5858.3ms)

ram: fluctuate between 220 and 238 Seems to be garbage collected every request

time curl: real 0m6.630s

With current pull request :

disconnected player

(only see lastest responses)

display view: I, [2018-07-24T22:46:14.597187 #29835] INFO -- : Completed 200 OK in 924ms (Views: 921.3ms)

ram: start at "111.44921875" stabilize at "116"

time curl: real 0m0.984s

Best player

(see all response)

display view: I, [2018-07-24T22:42:49.796688 #28369] INFO -- : Completed 200 OK in 1152ms (Views: 1147.5ms)

ram: start at "116.62" stabilize at "120.0273437"

time curl: real 0m1.120s

Hettomei commented 6 years ago

same benchmark, but in a table

ram page view rails note
dev: master; worst player 250..256 OK in 3078ms
dev: pull request; worst player 134 then grow 0.26 OK in 1077ms :1st_place_medal:
dev: master; best player 272..286 OK in 7287ms sometimes crash
dev: pull request; best player 134 then grow 0.26 OK in 1192ms :1st_place_medal:
prod: master; worst player 220..238 OK in 3139ms
prod: pull request; worst player 116 OK in 921ms :1st_place_medal:
prod: master; best player 220..238 OK in 6682ms
prod: pull request; best player 120 OK in 1147ms :1st_place_medal:
Hettomei commented 6 years ago

Interpretation of the benchmark : No need to go deeper, it seems that this PR improve vimgolf.

Hettomei commented 6 years ago

Again, ready to merge from my point of view ;)

igrigorik commented 6 years ago

Awesome work sir. Merging.