Service Provider Is Very Slow

[MannikJ]

$this->apiClasses = SellingPartnerApi::getSpApiClasses();

The code above to find all the api classes runs very slowly. It takes about 0.5 seconds on my machine each time it is run. Since the service provider is created at every framework boot, this has a huge performance impact. In fact we noticed that our test suite was slowed down drastically - it took basically 5 times longer only because the above piece of code. Not to mention that the service provider also tries to obtain auth tokens every time it runs like discussed in another issue (#2 ). This impact is not even factored it.

I can think of some quick fixes to improve the performance of this service provider related to class loading by also preparing the setup to be a bit more flexible for later improvements:

• Make the class registration optional (e.g. by config/env var), so it can be deactivated during tests. The user can then either create mock classes manually or just leave it activated as before. Mocking is another topic on its own and it would be great if this would also be baked into the package, but this is not subject of this issue.
• Move the class finding (snippet above) to a place where it is only executed when the classes are actually needed (class registration enabled). Currently the api class finding is done immediately in the class constructor. It should be moved to the register function instead.

The above changes would only speed up the tests by deactivating the class registration completely. In production, we can not use a steamroller approach like this, because the binding is actually needed, so that the app knows how to resolve these classes with their correct configuration.

To reduce the performance impact in production we could:

• either move the class finding code to the config file. This way the classes can be cached together with the rest of config, if config caching is enabled. In the service provider's register we could use the config helper to get these classes, it should be way faster when the config is cached.
• or remove the class finding and list all the classes explicitly
• change the singleton binding to a normal binding.
  I am not exactly sure why singleton was chosen for this matter, but in fact when binding something as a singleton, the app will immediately call the resolve callback to make sure the instance exists. I guess using normal binding should work just as well and maybe behave more like the user would expect. It would be more a on-demand creation than creating all the objects even if they are not used at all. Of course each call would resolve it's own instance which could potentially lead to issues, but I don't think it actually will.

If my suggestions feel valid to you, I could prepare a pull request. I'm looking forward to hear your thoughts on my ideas.

[jlevers]

Hey @MannikJ,

Thanks for the thorough report, and sorry I haven't gotten back to you on #2 yet. I've been super busy, so I probably won't have a chance to address it for a while.

Starting at the end first, there are two reasons I chose to use a singleton for API classes in single-seller mode:

1. Since there is only one set of seller creds in use, the same instance of the API class will work everywhere.
2. I wasn't initially caching access tokens, so by using the same API instance everywhere, we avoided fetching a new access token every time an API class was auto-injected.

I think these suggestions of yours are the best options for fixing a) the issue of tests being slow, and b) keeping things running fast in production:

• Move the call to fetch all the API classes to the register method
• Cache the list of API classes, but not in the config file, as I think it's good to keep the config file as logic-free as possible. That caching can happen in the register method as well.

If speed is still an issue after those fixes, manually listing all the API classes instead of auto-detecting them would be the next step, I think. The list of classes changes infrequently enough that I don't think that it would be a big deal to update them occasionally.

A PR would be much appreciated!

[jlevers]

Closed by #4.