Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Speedup peak spreading #32

Merged
merged 21 commits into from
Sep 4, 2022
Merged

Conversation

Dronablo
Copy link
Contributor

I tried recognize_song on different inputs and py-spy shows that most of the time was spent not on waiting response from Shazam itself, but on waiting for do_peak_spreading(). My timings was like '1.5 seconds in preak_spreading for 3 seconds of input or 6 seconds in preak_spreading for 10 seconds of input`.

So, here is a numpy implementation:

  • It do exactly the same: doesn't change any logic or class variable types.
  • It works ~10 times faster.
  • As most of [my] numpy code it's pretty less easy readable/understandable than pure python version.

@lgtm-com
Copy link

lgtm-com bot commented Aug 30, 2022

This pull request introduces 1 alert and fixes 1 when merging ff1080f into 3f3c7b9 - view on LGTM.com

new alerts:

  • 1 for Module is imported with 'import' and 'import from'

fixed alerts:

  • 1 for Unnecessary pass

@dotX12
Copy link
Collaborator

dotX12 commented Sep 1, 2022

@Dronablo, Please post tests before and after implementing the numpy library.

Andrey Chernopyatov and others added 2 commits September 3, 2022 12:04
@Dronablo
Copy link
Contributor Author

Dronablo commented Sep 3, 2022

@Dronablo, Please post tests before and after implementing the numpy library.

Done

@lgtm-com
Copy link

lgtm-com bot commented Sep 3, 2022

This pull request introduces 1 alert when merging 4f9e493 into 3f3c7b9 - view on LGTM.com

new alerts:

  • 1 for Module is imported with 'import' and 'import from'

@lgtm-com
Copy link

lgtm-com bot commented Sep 3, 2022

This pull request introduces 1 alert when merging a957803 into 3f3c7b9 - view on LGTM.com

new alerts:

  • 1 for Module is imported with 'import' and 'import from'

@dotX12
Copy link
Collaborator

dotX12 commented Sep 4, 2022

test:

async def tasks():
    a = [main(), main(), main(), main(), main(), main(), main()]
    res = await asyncio.gather(*a)
    return res

before: 6.474631299999601
after: 3.4023000999986834

realy good

@dotX12 dotX12 changed the base branch from master to dev September 4, 2022 03:42
@dotX12 dotX12 merged commit 87ec8b7 into shazamio:dev Sep 4, 2022
@dotX12 dotX12 mentioned this pull request Sep 4, 2022
@dotX12
Copy link
Collaborator

dotX12 commented Sep 4, 2022

@Dronablo, Thx!

@dotX12
Copy link
Collaborator

dotX12 commented Sep 4, 2022

@dotX12 dotX12 mentioned this pull request Sep 4, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants