You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@lucene.apache.org by GitBox <gi...@apache.org> on 2022/12/13 01:17:05 UTC
[GitHub] [lucene] rmuir opened a new pull request, #12014: Ban use of Math.fma across the entire codebase
rmuir opened a new pull request, #12014:
URL: https://github.com/apache/lucene/pull/12014
When FMA is not supported by the hardware, these methods fall back to `BigDecimal` usage [1] which causes them to be 2500x slower [2].
While most hardware in the last 10 years may have the support, out of box both VirtualBox and QEMU don't pass thru FMA support (for the latter at least you can tweak it with e.g. -cpu host or similar to fix this).
This creates a terrible undocumented performance trap, see [3] for an example of a 30x slowdown of an entire application. In my experience, developers are often too far detached from the production reality, and that reality is: we're not deploying to macbook pros in production, instead we are almost all using virtualization: we can't afford such performance traps.
Practically it would be an issue too: e.g. Policeman jenkins instance that runs our tests currently uses virtualbox. It would be bad for vector-search tests to suddenly get 30x slower.
We can't safely use this method anywhere, as we don't have access to check CPUID or anything to see if it will be insanely slow or not. Let's ban it completely: I'm concerned it will sneak into our codebase otherwise... it almost happened before: #10718
[1] [Math.java source code](https://github.com/openjdk/jdk/blob/5d4ea9b9549b762b7c207e5c2ee65bc51591433b/src/java.base/share/classes/java/lang/Math.java#L2364-L2366)
[2] [Comment on JIRA issue for x86 intrinsic mentioning 2500x speedup](https://bugs.openjdk.org/browse/JDK-8154122?focusedCommentId=13995171&page=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel#comment-13995171)
[3] [VirtualBox bug for lack of FMA support](https://www.virtualbox.org/ticket/15471)
--
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: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene] dweiss commented on pull request #12014: Ban use of Math.fma across the entire codebase
Posted by GitBox <gi...@apache.org>.
dweiss commented on PR #12014:
URL: https://github.com/apache/lucene/pull/12014#issuecomment-1351660443
I honestly don't know who can use this method without any provided cpuid check... We actually use fma in our code but do so by detecting the performance difference between a naive implementation on primitive types and Math.fma (during bootstrap). It's ugly like hell but the difference is so vast that it works. I'm not sure who'd even gain from using the bigdecimal-based implementation...
--
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: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene] gsmiller commented on pull request #12014: Ban use of Math.fma across the entire codebase
Posted by GitBox <gi...@apache.org>.
gsmiller commented on PR #12014:
URL: https://github.com/apache/lucene/pull/12014#issuecomment-1347617300
+1, seems reasonable to me. We can always remove this ban in the future if there's a good reason, but seems reasonable to put this in place to prevent it sneaking in 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: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene] rmuir merged pull request #12014: Ban use of Math.fma across the entire codebase
Posted by GitBox <gi...@apache.org>.
rmuir merged PR #12014:
URL: https://github.com/apache/lucene/pull/12014
--
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: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene] benwtrent commented on pull request #12014: Ban use of Math.fma across the entire codebase
Posted by GitBox <gi...@apache.org>.
benwtrent commented on PR #12014:
URL: https://github.com/apache/lucene/pull/12014#issuecomment-1351560532
Holy crap, creating `BigDecimal` and then multiplying & adding is crazy. This is a completely unacceptable fallback calculation for this method.
+1 on banning its use in the code base.
--
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: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene] uschindler commented on pull request #12014: Ban use of Math.fma across the entire codebase
Posted by GitBox <gi...@apache.org>.
uschindler commented on PR #12014:
URL: https://github.com/apache/lucene/pull/12014#issuecomment-1351734347
+1
--
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: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene] rmuir commented on pull request #12014: Ban use of Math.fma across the entire codebase
Posted by GitBox <gi...@apache.org>.
rmuir commented on PR #12014:
URL: https://github.com/apache/lucene/pull/12014#issuecomment-1347620183
Yeah, I think if the fallback java code was 2x, 4x, or 8x slower (like you would expect from these intrinsics), we wouldn't be having this conversation :)
--
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: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org
[GitHub] [lucene] rmuir commented on pull request #12014: Ban use of Math.fma across the entire codebase
Posted by GitBox <gi...@apache.org>.
rmuir commented on PR #12014:
URL: https://github.com/apache/lucene/pull/12014#issuecomment-1351680479
I looked at what e.g. glibc does here as a fallback out of curiousity, for floats it is very simple (using Dekker algorithm), but requires changing the FP rounding mode, which you cant do in java. For doubles it is more complicated but still no bigdecimal.
--
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: issues-unsubscribe@lucene.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@lucene.apache.org
For additional commands, e-mail: issues-help@lucene.apache.org