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.

Sign up for GitHub

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

Jump to bottom

Loom support. #1176

Merged

yschimke merged 17 commits into square:master from yschimke:loom

Jan 5, 2023

Collaborator

yschimke commented Dec 30, 2022

Towards #1169

For discussion

General approach from


          [Experiment] Loom support.

d2a4774

yschimke requested a review from swankjesse

December 30, 2022 12:06

Collaborator Author

yschimke commented Dec 31, 2022 •

edited

@swankjesse Are there any benchmarks we can test

master
yschimke:loom on JDK 9
yschimke:loom on JDK19 with loom enabled.

yschimke commented

View reviewed changes

okio/src/commonMain/kotlin/okio/FileHandle.kt Outdated Show resolved Hide resolved


          Simplify

16f8d45

Collaborator Author

yschimke commented Dec 31, 2022

TODO

add a CI build against Loom
test with -Djdk.tracePinnedThread=full

yschimke mentioned this pull request

Run tests on JDK 18 + 19 #1177

Merged

swankjesse reviewed

View reviewed changes

okio/src/commonMain/kotlin/okio/-CommonPlatform.kt Outdated Show resolved Hide resolved

swankjesse reviewed

View reviewed changes

okio/src/commonMain/kotlin/okio/-CommonPlatform.kt Outdated Show resolved Hide resolved

swankjesse reviewed

View reviewed changes

okio/src/jvmMain/kotlin/okio/AsyncTimeout.kt

@@ @@ -253,7 +259,7 @@ open class AsyncTimeout : Timeout() { @@
                           prev.next = node
                           if (prev === head) {
                             // Wake up the watchdog when inserting at the front.
-                            (AsyncTimeout::class.java as Object).notify()
+                            condition.signal()

Member

swankjesse Dec 31, 2022

Nice

swankjesse reviewed

View reviewed changes

okio/src/jvmMain/kotlin/okio/Throttler.kt

                     while (true) {
                       val now = System.nanoTime()
                       val byteCountOrWaitNanos = byteCountOrWaitNanos(now, byteCount)
                       if (byteCountOrWaitNanos >= 0) return byteCountOrWaitNanos
                       waitNanos(-byteCountOrWaitNanos)
                     }
                   }
-                  throw AssertionError() // Unreachable, but synchronized() doesn't know that.

Member

swankjesse Dec 31, 2022

Nice

swankjesse reviewed

View reviewed changes

okio/src/jvmMain/kotlin/okio/Throttler.kt Outdated Show resolved Hide resolved

swankjesse reviewed

View reviewed changes

okio/src/jvmMain/kotlin/okio/Timeout.kt Outdated Show resolved Hide resolved

swankjesse reviewed

View reviewed changes

okio/src/jvmMain/kotlin/okio/Timeout.kt Outdated Show resolved Hide resolved

swankjesse reviewed

View reviewed changes

okio/src/jvmMain/kotlin/okio/Timeout.kt Outdated Show resolved Hide resolved

swankjesse reviewed

View reviewed changes

Member

swankjesse left a comment

Love where you’re going with this.

Please take the next steps:

replace synchronized with withLock
tests for the function on Timeout

yschimke added 4 commits

January 2, 2023 16:23


          Merge branch 'master' into loom

5344e6a


          Review comments

a7d8ffd


          More fixes.

72899bf


          Add a test.

dade57f

yschimke changed the title ~~[Experiment] Loom support.~~ Loom support.

yschimke added 3 commits

January 2, 2023 17:26


          Fix formatting.

22516e5


          Run loom tests

1dbddda


          Run loom tests

ff74f80

yschimke marked this pull request as ready for review

January 2, 2023 09:28


          Cleanup

yschimke mentioned this pull request

Introduce Timeout.waitUntil(monitor, condition: () -> Boolean) #811

Open


          Merge branch 'master' into loom

6718e0c

swankjesse reviewed

View reviewed changes

okio-testing-support/src/jvmMain/kotlin/okio/TestingExecutors.kt

+              import java.util.concurrent.ScheduledExecutorService
+              import java.util.concurrent.ThreadFactory
+              object TestingExecutors {

Member

swankjesse Jan 4, 2023

This is rad

swankjesse reviewed

View reviewed changes

okio/src/jvmMain/kotlin/okio/Timeout.kt Outdated Show resolved Hide resolved

swankjesse reviewed

View reviewed changes

okio/src/jvmMain/kotlin/okio/Timeout.kt Outdated Show resolved Hide resolved

swankjesse reviewed

View reviewed changes

okio/src/jvmMain/kotlin/okio/Timeout.kt Outdated Show resolved Hide resolved

swankjesse reviewed

View reviewed changes

okio/src/jvmTest/java/okio/AwaitSignalTest.kt Outdated Show resolved Hide resolved

swankjesse reviewed

View reviewed changes

okio/src/jvmTest/java/okio/AwaitSignalTest.kt Outdated Show resolved Hide resolved

swankjesse reviewed

View reviewed changes

okio/src/jvmTest/java/okio/PipeTest.java Outdated Show resolved Hide resolved

swankjesse approved these changes

View reviewed changes

yschimke and others added 6 commits

January 5, 2023 09:10


          Update okio/src/jvmTest/java/okio/AwaitSignalTest.kt

a67f4fc

Co-authored-by: Jesse Wilson <jwilson@squareup.com>


          Update okio/src/jvmMain/kotlin/okio/Timeout.kt

bfd0795

Co-authored-by: Jesse Wilson <jwilson@squareup.com>


          Update okio/src/jvmTest/java/okio/AwaitSignalTest.kt

760daa8

Co-authored-by: Jesse Wilson <jwilson@squareup.com>


          Update okio/src/jvmMain/kotlin/okio/Timeout.kt

Co-authored-by: Jesse Wilson <jwilson@squareup.com>


          Update okio/src/jvmMain/kotlin/okio/Timeout.kt

29667df

Co-authored-by: Jesse Wilson <jwilson@squareup.com>


          Review comments

2d530d5

yschimke merged commit f8434f5 into square:master

yschimke mentioned this pull request

Loom Support #1169

Closed

wendigo mentioned this pull request

[JDK 21] Build and run Trino on JDK 21 trinodb/trino#17017

Closed

13 tasks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment