nickva commented on issue #5801: URL: https://github.com/apache/couchdb/issues/5801#issuecomment-3659498031
@Benjamin-Philip thanks for additional benchmarking and confirming the results. I liked the idea of generating the benchmark data separately and did that and replaced the script with a call to erlperf in a PR in https://github.com/apache/couchdb/pull/5824. Also, thanks for suggestion to use a proper benchmarking suite! Yeah, it seems hardly worth it to add a separate clause for < 100 bytes. I think that might speed up the change feeds but only for something like a `q=1&n=1` databases. With larger q values (q>4) we'be over 100 bytes already and it's almost not worth having an extra case in the code just for it. For inline attachment base-64 binaries could be arbitrarily large, so then the nif speed becomes more important. When decoding a 10MB binary we're talking about 20msec vs 60msec which may be noticeable by users. > You might also conclude that removing b64url doesn't make any meaningful improvement to the maintenance workload since b64url is so rarely updated. b64url does seem to make a difference with larger binaries, and like @davisp had mentioned the b64url nif has worked rock solid for more than a decade. To replace it we'd want the OTP version to be pretty close to the NIF performance, it may even be a tiny bit slower, but so far it's more than a tiny bit. So maybe it's worth benchmarking after a few more OTP releases, so I think that means we'd go with Option 2 for now. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
