android_jni: Set threads to 2 instead of CPU count #2472
Conversation
| * documentation for maxThreads variable in avif.h. | ||
| * @param threads Number of threads to be used for the AVIF decode. Zero means use the library | ||
| * determined optimal value as the thread count. Negative values mean use number of CPU cores | ||
| * as the thread count. When this value is > 0, it is simply mapped to the maxThreads |
There was a problem hiding this comment.
The sentence "When this value is > 0, it is simply mapped to the maxThreads parameter in libavif." is a little confusing. It seems to imply that the maxThreads parameter in libavif is only set when the value of the threads parameter is > 0.
There was a problem hiding this comment.
Removed the sentence about > 0.
|
In Chrome's avif_image_decoder.cc, I also set libavif's maxThreads parameter to the hardcoded value of 2. I have two reasons:
|
Yes, this change is also along the same lines. The only difference is that there are several low-end Android devices in the market which are single core. So i chose min(core_count, 2) as the default value instead of always 2. |
vigneshvg
force-pushed
the
cl_android_threads
branch
from
b53d542
to
1a90979
Compare
| * simply mapped to the maxThreads parameter in libavif. For more details, see the | ||
| * documentation for maxThreads variable in avif.h. | ||
| * @param threads Number of threads to be used for the AVIF decode. Zero means use the library | ||
| * determined optimal value as the thread count. Negative values mean use number of CPU cores |
There was a problem hiding this comment.
Previously negative values were invalid. I guess you want to provide a way to specify "use the number of CPU cores", so you now use negative values for that purpose?
There was a problem hiding this comment.
yes that is correct.
| * simply mapped to the maxThreads parameter in libavif. For more details, see the | ||
| * documentation for maxThreads variable in avif.h. | ||
| * @param threads Number of threads to be used for the AVIF decode. Zero means use the library | ||
| * determined optimal value as the thread count. Negative values mean use number of CPU cores |
There was a problem hiding this comment.
Nit: Add "the" before "number of CPU cores".
There was a problem hiding this comment.
Done. Also removed the comment about failure because of negative values to threads.
vigneshvg
force-pushed
the
cl_android_threads
branch
from
1a90979
to
33c9a4f
Compare
Empirically, on Android devices with more than 1 core, it is almost always better to use 2 threads than to use number_of_cpu_cores threads. Update the public java API function documentation to reflect this change. The new behavior is: * negative values map to number of cpu cores. * zero maps to 1 or 2 depending on single core or multi core devices. * value >=0 maps to value. This change ensures that users who use the library via JNI take the optimal path when using the default value of 0.
vigneshvg
force-pushed
the
cl_android_threads
branch
from
33c9a4f
to
530828c
Compare
Empirically, on Android devices with more than 1 core, it is
almost always better to use 2 threads than to use
number_of_cpu_cores threads.
Update the public java API function documentation to reflect this
change.
The new behavior is:
This change ensures that users who use the library via JNI take the
optimal path when using the default value of 0.