Skip to content

Conversation

Copy link

Copilot AI commented Dec 17, 2025

Addresses review question about thread-safety of Arrays.parallelSort usage in the single-column sort optimization path.

Changes

  • Added inline comment documenting why parallelSort is thread-safe:
    • Comparator performs concurrent reads only via column[index]
    • Column access is side-effect-free (values[index] delegation)
    • Standard List implementations support concurrent reads without modification

The parallelSort implementation is correct as-is. The locally-created permutation array and read-only column access satisfy thread-safety requirements.

val permutation = Array(column.size()) { it }
// parallelSort is thread-safe here: the comparator only performs concurrent reads from the column,
// and column.get() is a simple, side-effect-free access to an underlying List.
// Standard List implementations (ArrayList, etc.) are safe for concurrent reads
// as long as there are no concurrent modifications to the dataframe.
Arrays.parallelSort(permutation, finalComparator)

💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

Co-authored-by: zaleslaw <1198621+zaleslaw@users.noreply.github.com>
Copy link
Author

Copilot AI commented Dec 17, 2025

Just as a heads up, I was blocked by some firewall rules while working on your feedback. Expand below for details.

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

  • dl.google.com
    • Triggering command: /usr/lib/jvm/temurin-17-jdk-amd64/bin/java /usr/lib/jvm/temurin-17-jdk-amd64/bin/java --add-opens=java.base/java.lang=ALL-UNNAMED --add-opens=java.base/java.lang.invoke=ALL-UNNAMED --add-opens=java.base/java.util=ALL-UNNAMED --add-opens=java.prefs/java.util.prefs=ALL-UNNAMED --add-exports=jdk.compiler/com.sun.tools.javac.api=ALL-UNNAMED --add-exports=jdk.compiler/com.sun.tools.javac.util=ALL-UNNAMED --add-opens=java.base/java.util=ALL-UNNAMED --add-opens=java.prefs/java.util.prefs=ALL-UNNAMED --add-opens=java.base/java.nio.charset=ALL-UNNAMED --add-opens=java.base/java.net=ALL-UNNAMED --add-opens=java.base/java.util.concurrent=ALL-UNNAMED --add-opens=java.base/java.util.concurrent.atomic=ALL-UNNAMED --add-opens=java.xml/javax.xml.namespace=ALL-UNNAMED -Xmx16G -Dfile.encoding=UTF-8 -Duser.country=US -Duser.language=en -Duser.variant -cp (dns block)
  • packages.jetbrains.team
    • Triggering command: /usr/lib/jvm/temurin-17-jdk-amd64/bin/java /usr/lib/jvm/temurin-17-jdk-amd64/bin/java --add-opens=java.base/java.lang=ALL-UNNAMED --add-opens=java.base/java.lang.invoke=ALL-UNNAMED --add-opens=java.base/java.util=ALL-UNNAMED --add-opens=java.prefs/java.util.prefs=ALL-UNNAMED --add-exports=jdk.compiler/com.sun.tools.javac.api=ALL-UNNAMED --add-exports=jdk.compiler/com.sun.tools.javac.util=ALL-UNNAMED --add-opens=java.base/java.util=ALL-UNNAMED --add-opens=java.prefs/java.util.prefs=ALL-UNNAMED --add-opens=java.base/java.nio.charset=ALL-UNNAMED --add-opens=java.base/java.net=ALL-UNNAMED --add-opens=java.base/java.util.concurrent=ALL-UNNAMED --add-opens=java.base/java.util.concurrent.atomic=ALL-UNNAMED --add-opens=java.xml/javax.xml.namespace=ALL-UNNAMED -Xmx16G -Dfile.encoding=UTF-8 -Duser.country=US -Duser.language=en -Duser.variant -cp (dns block)

If you need me to access, download, or install something from one of these locations, you can either:

Copilot AI changed the title [WIP] Address feedback on thread-safety of parallel sort Document thread-safety of Arrays.parallelSort in sortByColumns Dec 17, 2025
Copilot AI requested a review from zaleslaw December 17, 2025 14:48
@zaleslaw zaleslaw closed this Dec 18, 2025
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