Skip to content
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

JDK 17 Upgrade #64

Merged
merged 5 commits into from
Mar 9, 2023
Merged

JDK 17 Upgrade #64

merged 5 commits into from
Mar 9, 2023

Conversation

SentryMan
Copy link
Contributor

It seems Jsoniter isn't compatible when I upgrade to JDK 17.

Screen shot of working lib:

image

Copy link
Owner

@fabienrenaud fabienrenaud left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removing jsoniter, a top-contender lib, so swiftly isn't great. Please report the issue to the jsoniter project and make sure to emphasize we may have to remove jsoniter from this benchmark if it isn't fixed soon.

Furthermore, I'd like to squash-merge all JDK 17 related changes, so please create separate (earlier) commits/PRs for changes that are not strictly related to the JDK 17 code (e.g. jsoniter removal; avaje-jsonb).

ami.json Show resolved Hide resolved
@SentryMan SentryMan changed the title JDK 17/Upgrade Avaje version even more/Remove Jsoniter JDK 17 Upgrade Feb 27, 2023
@SentryMan
Copy link
Contributor Author

Removing jsoniter, a top-contender lib, so swiftly isn't great. Please report the issue to the jsoniter project and make sure to emphasize we may have to remove jsoniter from this benchmark if it isn't fixed soon.

They seem to have a bunch of issues around it already, see this one for example json-iterator/java#332 don't think it's been resolved yet.

@SentryMan SentryMan requested a review from fabienrenaud March 8, 2023 23:35
@fabienrenaud fabienrenaud merged commit 9597378 into fabienrenaud:master Mar 9, 2023
@SentryMan
Copy link
Contributor Author

by the way, when we expect the next run results?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants