paulirwin commented on issue #1050: URL: https://github.com/apache/lucenenet/issues/1050#issuecomment-2501924301
> These "who called me" checks exist in Lucene for a reason. What is that reason? If anything, the comment on it indicates a keenness to remove the assertion, rather than keep it, because it acknowledges that it's testing an internal API. Users should only be concerned that the exception was thrown as expected, not that a specific protected method in the call stack was the one that did the check. Perhaps we could ask the Lucene team for their thoughts, but my gut tells me that if they had anything as awesome as Dynamic PGO at the time, they would lean on the side of removing this assertion rather than harming performance by forcing it not to be inlined, just to keep the assertion in the test (which has no meaningful benefit to end users). Remember that Dynamic PGO is the runtime observing ways that it can tangibly improve performance based on real-world measured behavior, and then making those changes. By inhibiting this with NoInlining, we're choosing to harm performance just for purity with the original assertion. IMO Dynamic PGO's inlining is a feature, not a bug. And I think we should do a sweep to remove all such assertions and NoInlining that was added to make these assertions pass. Let's err on the side of letting the runtime give us better performance, than having to make sure our stack trace matches some decade+ old version of the JDK's behavior. Especially when that assertion's author was even questioning whether it should be there in the first place. We could be considered the "later version" of this code that is changing the internal API, at runtime, via Dynamic PGO. -- 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: dev-unsubscr...@lucenenet.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
