vrp benchmark

[braktar]

As mentioned on the discussion forum, we are currently still using v0.18.0 as the performances for large models are, in the recent versions, greatly slowest.

This PR add a benchmark test with a large model.

With version v0.18, the input is treated in 13.934s

With version v1.4 and the default param builder : 88.520s
With version v1.4 and the hash param builder : 39.292s
With version v1.4 and hashie mash as param builder : 108.923s

[dblock]

Thanks for the benchmark. If you can, please post the associated profiler outputs in a gist or here.

I might take a look, but making no promises. This is an interesting problem to solve.

[dblock]

https://groups.google.com/d/msgid/ruby-grape/47ace8cc-18e8-4fd3-9ec3-749f2492c74an%40googlegroups.com?utm_medium=email&utm_source=footer

[braktar]

Here are the time to process the instance through multiple versions of grape

version	time(s)
0.18.0	5,8
0.19.2	5,8
1.0.0	3,9
1.0.1	68
1.0.2	72
1.0.3	68
v1.1.0	73
v1.2.0	79
v1.2.1	81
v1.2.2	79
v1.2.3	82
v1.2.4	73
v1.2.5	90
v1.3.0	error
v1.3.1	error
v1.3.2	error
v1.3.3	87
v1.4.0	90

Moreover, here are the time profile for versions 0.18.0, 1.0.0, 1.0.1 and 1.4.0
https://gist.github.com/braktar/74a97be4a77a745262fb6bb739d5777a

[dblock]

I see a bisect between 1.0.0 and 1.0.1 as a next step!

[braktar]

I not really sure of the logic behind the meets_dependency? method. My assumption is that currently a param checks that all its child meet its own dependency, even before checking that it requires to really check it ?

Btw, changing the order makes the benchmark met a total time around 3 seconds

[dblock]

I not really sure of the logic behind the meets_dependency? method. My assumption is that currently a param checks that all its child meet its own dependency, even before checking that it requires to really check it ?

Btw, changing the order makes the benchmark met a total time around 3 seconds

I don't know much about that change, git blame and cc them here for comment?

[dblock]

The fact that all specs pass tells me that it's a fix. Try seeing which specs break if you remove the parent check, it should give us a hint on why that's there.

Update CHANGELOG, lets try to convince ourselves that it's correct and merge?

[braktar]

parameter @dependent_on is only active if the params or one of its parent belongs to a 'given' declaration.

Until now, we check that all the current params and their childs meets their dependency which first consist into verifying that one of the parent has the variable @dependent_on active.

I've added a spec to validate my assumption.

@rnubel as you've introduced the related change #1661 and #1625 of the meets_dependency? method. Is the logic correct ?

[dblock]

I merged #2097, care to rebase this please?

Can I please have another pair of eyes from @dm1try @dnesteryuk too?

[dblock]

and + @wanabe, probably coincidental but #2097 deals with dependencies

[braktar]

The branch is rebased and the conflict has been managed

I didn't find any interference in the grape behaviour with #2097

[dblock]

This is great work @braktar, thank you. I merged it.

[braktar]

Thanks, that's a great step for us ! It divides by 3 the time to compute our current biggest instances.
Moreover, it will allow us to update grape-swagger and finally fix the doc ! 🎉