-
Notifications
You must be signed in to change notification settings - Fork 3.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
avoiding NoSuchMethod exception #6658
Conversation
@aardappel can we publish 2.0.1 for Java after merging? |
I'd like to first understand what is going on here, as this doesn't seem to be the same problem as mentioned in your link. In most of these cases, Afaik sub-super class casting in Java has a significant runtime cost, and this is being called all over the place. @paulovap any insights? |
Android java cannot handle covariant return types in those methods and simply throws, since it cannot find the one with appropriate return type. I'm not sure why it started manifesting in flatbuffers 2.0, rather than sooner, but I can confirm it does happen and the suggested fix helps. |
Which return type? We don't use the |
Maybe let's continue the discussion in the bug report. We seem to be doing it in two places. |
Seems like there's an environment problem with one build. |
@aardappel any chance of merging? |
@@ -154,7 +155,7 @@ public void setDouble(int index, double value) { | |||
|
|||
@Override | |||
public int writePosition() { | |||
return buffer.position(); | |||
return ((Buffer) buffer).position(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not needed?
Merged it, to stop the breakage, though there is still one unnecessary cast in there. |
@aardappel this is an internal change, so we can publish 2.0.1 for Java. But first, can you remove that one extra cast? I don't have write permissions and going though whole PR process to change one line is a bit too much for me today :) |
ok, pushed a 2.0.1 release thru Maven.. but will require some time to show up as always. Once it does I'll do the gRPC ones. |
and gRPC seems to also have gone thru. |
With the changes introduced on google#6729, google#6671, google#6658 it seems that using java compiler version 8 is no longer possible. We are adjusting our CI build for Kotlin to use Java 11 as compiler, while still emiting 1.8 bytecode. The Kotlin CI action is also being breakdown into two: Mac & Linux. Kotlin mac will test ios & mac builds while Linux will test js and JVM. This change will improve build times for Kotlin on CI, which is currently taking long times.
With the changes introduced on #6729, #6671, #6658 it seems that using java compiler version 8 is no longer possible. We are adjusting our CI build for Kotlin to use Java 11 as compiler, while still emiting 1.8 bytecode. The Kotlin CI action is also being breakdown into two: Mac & Linux. Kotlin mac will test ios & mac builds while Linux will test js and JVM. This change will improve build times for Kotlin on CI, which is currently taking long times.
This reverts commit 813d363.
* Revert "avoiding even more NoSuchMethod exceptions (#6729)" This reverts commit 6fb2c90. * Revert "avoiding more NoSuchMethod exceptions (#6671)" This reverts commit 752c7b5. * Revert "avoiding NoSuchMethod exception (#6658)" This reverts commit 813d363. * Use Java 8 for Kotlin Linux builds to verify
fixes #6657