Skip to content

[data] Chunk fixes & benchmark#1001

Merged
fwbrasil merged 1 commit into
getkyo:mainfrom
hearnadam:chunk-optimizations
Jan 13, 2025
Merged

[data] Chunk fixes & benchmark#1001
fwbrasil merged 1 commit into
getkyo:mainfrom
hearnadam:chunk-optimizations

Conversation

@hearnadam
Copy link
Copy Markdown
Collaborator

No description provided.

*/
final override def isEmpty: Boolean = length == 0

final override def knownSize: Int = length
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

This is quite important where other libraries may pre-allocate based on knowSize.

* a new Chunk with the last n elements removed
*/
override def dropRight(n: Int): Chunk[A] =
final override def dropRight(n: Int): Chunk[A] =
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

It's easier to remove final than add it (for bin compat). We can change later if we need to override in a subclass.

self match
case c: Indexed[B] @unchecked => c
case _ => Compact(toArrayInternal)
case _ => Compact(toArray)
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

This avoids matching on Indexed again

case seq: IndexedSeq[A] => FromSeq(seq)
case _ => Compact(source.iterator.toArray(using erasedTag[A]))
case _ =>
val array = source.iterator.toArray(using erasedTag[A])
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Iterators may produce an empty array. This shouldn't be an issue for Seqs.

Comment on lines +77 to +79
"Indexed" - {
"returns the same instance for Chunk.Indexed input" in {
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

We should probably have a Chunk.from that doesn't automatically compact in the main Chunk object.

@@ -0,0 +1,243 @@
package kyo.bench
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

I didn't use this to optimize anything yet, though it should be helpful.

def setup(): Unit =
val intArray = Array.fill(size)(intFillFn)
intKyoChunk = Chunk.from(intArray)
intZioChunk = ZChunk.from(intArray)
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

ZIO's Chunk.from doesn't copy arrays. Seems quite dangerous.

@hearnadam hearnadam force-pushed the chunk-optimizations branch from fcfcefc to ad741f4 Compare January 12, 2025 23:49
@fwbrasil
Copy link
Copy Markdown
Collaborator

it seems equality between different chunk types is broken: https://github.com/getkyo/kyo/actions/runs/12738165985/job/35500083525?pr=1001#step:6:4338

@hearnadam hearnadam force-pushed the chunk-optimizations branch from ad741f4 to 26adccf Compare January 13, 2025 05:17
@hearnadam hearnadam force-pushed the chunk-optimizations branch from 26adccf to 4672a40 Compare January 13, 2025 05:56
@fwbrasil fwbrasil merged commit 9cdf197 into getkyo:main Jan 13, 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