On Fri, 27 Jun 2025 13:24:52 GMT, Chen Liang <li...@openjdk.org> wrote:
>> But the original code already checks for `len >= 0`, right? See >> `LibraryCallKit::inline_countPositives` -> `generate_string_range_check` -> >> `// Offset and count must not be negative` >> >> This PR is about moving the range checks from the intrinsics into the Java >> wrappers. Removing range checks is out of the scope and should be carefully >> evaluated on a case-by-case basis separately. > > My point is this is a performance-sensitive API. We are using a known-slow > check method `checkFromIndexSize` which may introduce a performance > regression. @liach, thanks for sharing your feedback on this draft, much appreciated. 🙇 In order to address your (rightful) concerns (which I also share) regarding the performance impact, in 196fc5d, I've added `StringCodingCountPositives` JMH benchmark and produced the following results: *OS:* Ubuntu 24.04.2 *CPU:* Intel Core Ultra 9 185H x 22 Mode Cnt Score Error Units avgt 10 8.617 ± 0.646 ns/op # 149882416a9 (strIntrinCheck) avgt 10 8.787 ± 0.313 ns/op # 5a1301df195 (master) AFAICT, (even though the patched version appears to be performing better 😛) the scores do match. Note that I will run the entire `test/micro/org/openjdk/bench/java/lang/String*.java` suite on various platforms and make sure there are no (visible?) performance regressions, before promoting this draft to a PR. Let me know if this addresses your concerns and if there is anything else I can do to better assess the performance impact. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/25998#discussion_r2185582087
