stanford-oval / thingpedia-common-devices

Thingpedia interface code for commonly used devices
Other
38 stars 28 forks source link

Spotify skill makes too many API requests #296

Closed gcampax closed 3 years ago

gcampax commented 3 years ago

Now that the NLP is backed by GPUs and it's reasonably fast, it's become more apparent that another source of perceived slowness in the Spotify skill is due to too many API requests. We should cache repeated API requests in a very short time-frame, such as repeated requests to the get_active_devices endpoints. This will help us both with speed and with API quotas.

ryachen01 commented 3 years ago

Just to clarify do we want to cache API requests just within one function call or throughout multiple function calls? If the former, do you have an example of when we make a lot of unnecessary API calls, and if the latter, how do we determine when to reset the cache?

gcampax commented 3 years ago

I think we should cache API requests across multiple Thingpedia function calls. We can use a timer to expire the cache.

Now, I am not sure the issue is that we make too many requests, or that we make unnecessary requests. I though get_available_devices was one that we keep hammering, although that might be only if we don't have a device at all, and we're playing a lot of songs at once.

Regardless I think one place where we make unnecessary calls is music_by_search:

All calls but the first one are not as necessary if we're playing music (we just need the name and ID, we can forego sorting by popularity). In get_playable, the hints parameter has a projection property with the list of output parameters that need to be filled. If some of the expensive fields are not listed, we can avoid making API calls to retrieve them.

ryachen01 commented 3 years ago

Ok that makes sense, I see a couple other places where we make some unnecessary calls. I think that for music_by_search, however, we still need popularity because there's a lot of scenarios where there are multiple instances of songs having identical names and we use popularity to determine which song to play. Also, I feel like the only api call we could cache would be get_available_devices, but I'm worried that by doing that we'll cause a lot of bugs when people close the Spotify app or switch between devices. One thing that is happening though is that we call get_available_devices twice when playing a song, so that needs to be fixed, but I think overall it's not the main cause of slowness.