Skip to content

Bring back the addMetrics optimization of LinkedQueue to improve performances of Queue.unbounded#9762

Merged
guizmaii merged 2 commits intoseries/2.xfrom
optimize_unbounded_queue
Apr 9, 2025
Merged

Bring back the addMetrics optimization of LinkedQueue to improve performances of Queue.unbounded#9762
guizmaii merged 2 commits intoseries/2.xfrom
optimize_unbounded_queue

Conversation

@guizmaii
Copy link
Copy Markdown
Member

@guizmaii guizmaii commented Apr 8, 2025

@guizmaii guizmaii self-assigned this Apr 8, 2025
@guizmaii guizmaii marked this pull request as draft April 8, 2025 08:13
@guizmaii guizmaii force-pushed the optimize_unbounded_queue branch from faa2fea to eb77066 Compare April 8, 2025 08:15
…performances of `Queue.unbounded`

See also:
- #8784
- #8745
@guizmaii guizmaii force-pushed the optimize_unbounded_queue branch from eb77066 to 7745a99 Compare April 8, 2025 08:21
@guizmaii guizmaii marked this pull request as ready for review April 8, 2025 11:39
@guizmaii guizmaii requested review from ghostdogpr and kyri-petrou and removed request for ghostdogpr April 8, 2025 11:40
override def size(): Int = jucConcurrentQueue.size()

override def enqueuedCount(): Long = enqueuedCounter.get()
override def enqueuedCount(): Long = if (addMetrics) enqueuedCounter.get() else 0L
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I guess this is very minor, but in order to avoid adding addMetrics as a class field, you could do this instead (and everywhere else I guess):

Suggested change
override def enqueuedCount(): Long = if (addMetrics) enqueuedCounter.get() else 0L
override def enqueuedCount(): Long = if (enqueuedCounter ne null) enqueuedCounter.get() else 0L

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Done

@guizmaii guizmaii force-pushed the optimize_unbounded_queue branch from 0c7ec98 to a9b9edd Compare April 9, 2025 00:10
*/
private[this] val enqueuedCounter = new AtomicLong(0)
private[this] val dequeuedCounter = new AtomicLong(0)
private[this] val enqueuedCounter = if (addMetrics) new AtomicLong(0) else null
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

@kyri-petrou Do you know why we need private[this] and not just private? 🤔
private[this] is something dropped from Scala3 anyway: https://docs.scala-lang.org/scala3/reference/dropped-features/this-qualifier.html#

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The code compiles differently between Scala 2/3. In Scala 2 the compiler will generate a private accessor method when you use private but not when you use private[this]. Basically private[this] in Scala 2 compiles the same way as private in Scala 3

@guizmaii guizmaii requested a review from kyri-petrou April 9, 2025 00:59
@guizmaii guizmaii merged commit 930437a into series/2.x Apr 9, 2025
18 checks passed
@guizmaii guizmaii deleted the optimize_unbounded_queue branch April 9, 2025 09:39
khajavi pushed a commit to khajavi/zio that referenced this pull request Apr 10, 2026
…performances of `Queue.unbounded` (zio#9762)

* Bring back the `addMetrics` optimization of `LinkedQueue` to improve performances of  `Queue.unbounded`

See also:
- zio#8784
- zio#8745

* Review: prefer `<field> ne null` over using `addMetrics`
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