Cache getRestMethodByName to improve performance of remote invocations

[Traksewt]

This function in RestAdapter is slow due to creating RestMethods each time. This can be cached initially, and rechecked if the entry was not found.

Description

getRestMethodByName was identified as slow performing when syncing many items across many models. The reason is that each call will iterate through all classes, and create new RestMethods which is slow.

This optimisation will create a name->RestMethod cache map which is initialised the first time it is called. This way, the look ups are fast. If there is a cache miss, then it will try to refresh the cache (if it hasn't been refreshed already).

Related issues

• None

Checklist

• [x] New tests added or existing tests modified to cover all changes
• [x] Code conforms with the style guide

[slnode]

Can one of the admins verify this patch? To accept patch and trigger a build add comment ".ok\W+to\W+test."

[slnode]

Can one of the admins verify this patch?

[slnode]

Can one of the admins verify this patch?

[slnode]

Can one of the admins verify this patch?

[bajtos]

@slnode ok to test

[bajtos]

Hi @Traksewt, thank you for the pull request. If my understanding is correct, then you are using strong-remoting to remotely invoke LoopBack server and this patch should make such invocations faster. Is that correct?

You proposed patch looks reasonably in general. My main concern is cache invalidation: in strong-remoting, it's possible to disable remote methods on the fly, it also may be possible to change remoting metadata too (at least in theory). I think the caching solution should be prepared to handle such situations, e.g. by invalidating the cache entry when SharedClass.prototype.disableMethodByName was called.

Thoughts?

[bajtos]

Last but not least, please rebase your feature branch on top of the latest 2.x. I enabled coveralls code coverage in #402 to allow us to better review which parts of the new code may be missing test coverage.

[Traksewt]

Hi @bajtos. Our use case is that we are using the offline sync on a cordova based mobile app and syncing to a cloud server. As we are moving towards production, we have noticed some performance and scale issues with sync that we have been working through.

I did the changes requested and added some more unit tests.

[Traksewt]

Hi @bajtos I've made the changes. Please review.

[Traksewt]

@bajtos changes done. please review.

[bajtos]

@Traksewt thanks. To speed up the review process, I have simplified and clean up your code in 634f646. PTAL and let me know if you are ok with my changes.

[Traksewt]

@bajtos sure. The tests look neater.

[bajtos]

Great! I have squashed all commits into a single one and will land this pull request after CI builds pass.

[bajtos]

Landed, thank you for the contribution!