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

Avoid re-making the DCT table if it is already correct when initialising #241

Merged
merged 1 commit into from
Aug 17, 2023

Conversation

AlexHarker
Copy link
Contributor

@AlexHarker AlexHarker commented Aug 16, 2023

This PR is the one remaining change from #172 that was not effectively integrated.

I will need to find a patch that demonstrates the speed issue to see if this is a worthwhile optimisation, but it also needs checking for anything that uses DCT. The behaviour to check is that around anything that causes an object to have init() called (so does the object produce the correct results on a second analysis for example)

@AlexHarker AlexHarker mentioned this pull request Aug 16, 2023
@AlexHarker
Copy link
Contributor Author

As expected the massive init() cost on DCT is making the table. Here's a way to test this PR in max for speed against the old version:


----------begin_max5_patcher----------
1587.3oc0Y0riaaCD9r2mBBgdnEXigI0utW5FTTf9DzdHnvfVh1KSnnTEo1c
SCRd1K+S1R1Rdo20NAqOHAOjTyLeyvYFN7K2LKXc0SDQ.3WAe.La1WtY1LCI
MgYt+OKnD+TNCKLSKHuprjvkA2ZGSRdRZnilC9ypGArJ91eqaPdaIkyHRyJg
NhzBy7qV+w2sr2DqZkcybgiZMVleOkucUCIWZkQDb9haAgvT8KX7B8Kj5I3e
zK4q2bi9wsuRUANG76LZ9m.XdAPPXJtC1TwJHMfpM.AtrlQDfeVHwMRfnDyX
+xI0X8GNuhU0X0AiJnjYHLMIJcQbVFJLNbTRPslcLtkc93VjAwBQF.KI8pfa
kTFiJH4U7Bgut.n3tYtohKEz+iXlX37EmTE0SliKMSN38MTLKXZkGhr9IIw5
WYgl+.egJ+FkGdaoupGLYub0njWIoYEgiWyHGpMOqpibTsjjetlX0tffaAAq
w7sA6bUNKzwtkBkrT+J05YD1yuaSUSI1L2jW.bwIOpvgibUpA40sRZIoYTjD
cLRNteBZBvBNEXoLeZk40fTvk1fOlWHTOnxrFRiCNb3ghITF4ARifVw6IayB
v008HOq2Rzf3GswKxtcGIJ2RJdGoFxCzt0umJtQI4RkX21XUrmRhB1+YpTAx
3sTinXIpLmNQxX3z.hnFmaWr191M7drHFY1LAyLQTfKc9Mo6QCkmwVVU9mHE
8b0UVjZBmxqaHBUHDrzI76FtfrA2xjqFXkgyiWFCyRV18arYuwIvi9oF0FOK
XaCsnhqEoAFFM4NlqBeX0z39plYFbb8HKV4rpPoIFTkyP1JViaz1MWT.T2fx
pJ1vg1sNFYizMbMkyO.SkU0SOXCc68mXsqqTCVdpusYDwpVtczUJWD4JA9gg
nsTkFzsYe3m+ILmpBfPz61Mp6hcCZiDduHuohwFnu1QdXjQJTt74jGoEx6ML
ZdO6sZ5z5NWpfcV4B5VhPNjlDuULjhP9YKn2iT6Z2V5URhJquRKFNA0lEpPJ
tu5QgahcNZ8Af8EX0eKd+HmCnePDTaHrc6emHoyQIdNXAGmCcpnbgQF28TaZ
x3tm678MgIbU.n4FkqRp8.lAnbPo.PEfFRcUijT.T3AoubP4Ejm5E2wkMwEB
5EhOCxvLLKyZ.7bwMT+E3UPnIS9LdBnAYpOcNnIqmYgIZaZlw1bPsbWeTUk4
NWGX+bg1vqMzNL29qBagKL6BhS9dCtuSEQaRfEMNvF8V.XcgUhBcmCI96Kt9
BBED+FHTPT1awHAIuEbXUmXN9ZGHvfTShdKFG8RCtTtaSG+y1t.Ox9q+n.gp
.TAPUfm5aYZZhsB.Uw.tZCFsH.zOZfL6ZBj1clmKNh0nlCHOI3MVET6OdBix
mpnSinpGebTUT01j2onNPBLTrU0YKo7cGc6C6hVB7cyw4JCodJCnqnLj3oLn
RFCfWIYH1SYH4JhCQdJCvqnLDdF1hqkLf7TFBmTFbD6Z9Rf9H0EqrsOXEVJa
nqak1sw86lzYcLW+ZixVV0ZL6fSvN1YkuYuJbY5CnDrFv.q8tcpi236vo51m
IR8si0hzwR8sLzV5uIZczh8s26xouMJQj3tLAe67ILLM376HrOppqUlIKeUp
pxKUp7E80DFF6SKw8rct9ZTQYygQnr3T2yjTEMSkc5mWRKrtkeEppKq1ZLy9
CKIAu119Os5mZboWdcboI4sMB5CD2Uj82u+u9C+05nWfVS4Ru1HmYJZG5p95
kp1kDg.ukLhdWyv4DvOA8ci7DV3IctOoRhhbMnKZ9x9d1HjZCc70vPutcyFR
y2.0Tkvrh58N9y0DaO7lOd2cXfxdmsGAhTjUN6vqAFrg0RKlqPBCH7Mvc15B
1gIf6vrsUMT48k.D3tVNUppF7tMajBkNpjdgJwxxDPLDAfIgYQf6TrcSC4e0
2l8hN9oqCIup0d1.juWk5kNKQn8lOBCMGkIbg4pdBiO.QMeggm3vBvGVQkCl
Ock0OekbGYH8kOwdvGXxEfQti.bRFgt.7wTXvyxojKBm7wHAuHbxKyT3kQqh
7hWoWBd4Amh2ezwWEePdvmKg9.eVj63ss1nEGby2ZlbvMdevsce7McO8sbe3
MbaNf0T2r8Me8l+2suNia
-----------end_max5_patcher-----------

To me this points to a need to see if this can be sped up anyway, as the cost of the init()(which I think runs once per file) massively outweighs the whole analysis itself - this is bad, but we might as well avoid it for now when it isn't even required (as the table is already the correct size/contents).

@AlexHarker
Copy link
Contributor Author

FWIW my numbers on one particular folder are:

Before PR: c 110,000 ms
After PR: c 9,000 ms

@AlexHarker
Copy link
Contributor Author

Testing this too:


----------begin_max5_patcher----------
1572.3oc0YsziaaCD9r8uBBgdnEXigod6doInn.8WP6ghBCZIJuLghRUjZ2M
MH42d4KYKYK4k9UJ18fL1gOlY9lgyLb3WlOyaS0KXtG3mA+EX1ruLe1LMIEg
Y1+elWI5kLJhqmlWVUYIlI7dvLl.+hPS2eA32qdFPqXa+ktAYskDFEKzqDZI
Rx0yuZyGe2pdSrpUzMykVp0HQ1iD110M3LgQF8gKV9.H.ln9AFsT8iu7K3uU
K4qymq97vUpJvEfekRx9D.wxAbLUxcPQEMG2.pJ.bTYMEyA+HWfZD.dIhR+o
SpwpMNqhV0XzAsJHkYHLINLYYTZpePTvnjfJM6XbK87wsPMhE3qAr3j6BtUR
nTBGmUwx4t5B3G0MyhJlfS9WrdhAKVdRUTMYFpTOYuOzPPTuoUdnuwOINR8S
Zf9efWnxWH8vaKcU8fw6kqFo7JvMqwLzFJ9Ps4UUceKUCIwmqwFsyy6Af2FD
aq2NWkyBcLGo7iWo9Iw3YDzyuqnpoDoma7E.WL7yRb3HWkZPVcqfThaFEI8O
FIG2OweBvBNEXIMeJk4ZPJ3JSvG8O998fJ8ZvMV3vhGRlPn3mvMbREqmrMyC
UW2i7rdKQAhezDuH8gcjHLConcjZvOQ5V+dpnFojKjhcaiQwdINza+1TICjw
ZIZQwPTZNshj1vo.DdMJyrXk8sa38XQju9vDLUGQAtx52jrGMjdFaoUYeBm2
yUWZQpwLBqtAykgPPBqvua3bbApkJVOvJCWDsJBlFup6uwlcgUfGcqF0FOya
aCIuhoDoAFFE4NlKCeXzzn9pldFLT8HKV5rJQoIFTlyPzx2fZT1MaT.+tAEU
UzgCsacTbgvNbMgwN.SEU0SOXCY6imXsapjCVdp8VOBecKyL5ZoKhXMG8zPz
VHSCZOrOb6eAwHx.HX0ocs5tb2flHgOxyZpnzA5qYjmFYjboKeF9YRt3QMiV
zydKmNotykxamUNmrEyECoIPa4CovEe1.58H0twdjds.Ky5K0hgSPdXgvE7G
qdlamXmiVe.XeAV8Oh2Ox4.5GDA0DBa242IR5bThmCVvw4PmJJWPn1cOwjlL
p66NeecXBaE.JtQXxjZOgn.BCTxADNnAWW0Hv4.Idf6KGDVN9kdwcrYSrgft
P7YPFlgYY1.fmKt42eANEDZxjOim.ZPl5SmCZx5YVpi1ljpsMGTK28GUkYty
TA1OWnM3dCsCyseUXKbo9TPT72av8cxHZSBr9iCrguE.VaXkv.68Ph99hqWP
nfn2.gBBSeKFIH9sfCq7FyQ26.AZjZRza43nWh2sxca53el1E3P1e0lB3xBP
4.YAdx8R2zDSE.xhAr0FLZQ.9+eCjo2Sfzbx7bwQjB0r.4IAuwpfZ+0SnD1T
EcpEU03iip7p1lrNE0VPDXnXKqyVPX6t51esK6Nv0CGmqLD3nLDdGkgPGkA3
cTFhbTFhuixP7YXKf2IYHwQYv+NhCoNJCQSJCVhcMewSck570l1GrFIDMjMs
Byw39cS5rtlqasQYKsZChdvMXG6txy2qB2l9.J.a.TvFmam53M9NXpt8oiT+
vXsHcrTeqBLk9qiVGtbe68tc5aiTDw1GSv0NeBCR7N+NB6hpZakY7pqRUkdo
BounqlvfHWZItisy0Uipe5BXneZTh8abhjltxN02aoEV0xubY0kUa0lY2gkX
uqss+Sq9IZW5U2GWZbVaCm7D19DY+4G9ieycsN7BzZBS3zA4TcQ6Pa0WWpZW
h4bzV7H5cMEkgA+.z0CxSXgmz49jJoensAcgKV02y12WdfN5dXn2zVTfa9Fn
rHKaMw4C7mqE1b2MWbt6f.o4NcO.DJIK80g2CHnf1RxWHABEF7Mv6MUEzgHf
2WTH3RkQJlbYBjUwfHnO.FGjFBdub+KZv+i5UqW1swp5MxpZM2Av20mL8VmM
Hv7BGAA5qrDrT+jNAQG.c5cX3MKLH4gUNYwyQqfFtSNd0J1Nxh4JeFTg5qeS
gKlOPGzGc.taBm7eENEci3yRG3C7Z4SPnCbRW2y0qSAQNwq3aBubw2CdS3Th
Cde2BcxIz6XFYhVbvKbq3wAur8Aup8wun8zul8gujs9hTS8B1y+57+C.ilux
A
-----------end_max5_patcher-----------

@tremblap tremblap merged commit f6ec9be into flucoma:main Aug 17, 2023
3 checks passed
@AlexHarker AlexHarker deleted the faster-pitch-init-rebuild branch January 19, 2024 10:08
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.

2 participants