Specialize ByteIterator in BinaryFen

[som-snytt]

Noticed in Scala community build.

The iterator tries to inline to the underlying without much success.

The latest Scala will rewrite unused imports, so that's a second commit.

As a footnote, "bare iterators" should normally extend collection.AbstractIterator to save on members.

[som-snytt]

It's interesting that the import in the test of the extension just works, competing with the implicit def in predef.

[som-snytt]

I don't resist scalafmt per se, but one-lining an extension makes it extend longer. The result is that I remove explicit types to make lines shorter.

Appeasing scalafmt

Maybe making the variables private would look better. I don't have a lot of experience with scalafmt.

[lenguyenthanh]

Thanks @som-snytt this looks great ❤️

We have benchmark for this BinaryFen, so I'll run it to see the result

The iterator tries to inline to the underlying without much success.

Just wonder what scalac otions do you use to notice such a thing.

I don't resist scalafmt per se, but one-lining an extension makes it extend longer. The result is that I remove explicit types to make lines shorter.

I think this is a good call, I don't agree with scalafmt a lot of time, but it's good to be consistent with code style.

Maybe making the variables private would look better. I don't have a lot of experience with scalafmt.

I think we can make this class IteratorOfBye private but it shouldn't master much.

[lenguyenthanh]

There is no different in the the benchmark within margin of error, maybe jit compiler takes care of the inlining things?

master:

[info] Benchmark              Mode  Cnt        Score       Error  Units
[info] BinaryFenBench.read   thrpt   45  5550976.025 ± 41616.736  ops/s
[info] BinaryFenBench.write  thrpt   45  3997816.760 ± 29362.134  ops/s
[info] Benchmark result is saved to jmh-result.json

Pr:

[info] Benchmark              Mode  Cnt        Score       Error  Units                                                                                                                                               │
[info] BinaryFenBench.read   thrpt   45  5496608.257 ± 44021.886  ops/s                                                                                                                                               │
[info] BinaryFenBench.write  thrpt   45  4014106.685 ± 14020.498  ops/s                                                                                                                                               │
[info] Benchmark result is saved to jmh-result.json

[som-snytt]

Anything is possible with runtime.

I noticed because of a bug with -Wunused that made me try the project and that showed another bug
scala/scala3#22507

This was more of a learning exercise about array iterator. Normally I would not optimize without measuring.

I noticed in the discussion about future inline trait they are casual about unspecialized interfaces, so probably they lean on the JIT.

[som-snytt]

I also don't see a change in the benchmark. I'll check using AbstractIterator, which has the fixed benefit of a smaller class file (which makes it easier to examine the bytecode of next and hasNext; though all that matters is the jitted code!).

Edit: as expected, AbstractIterator can be used without affecting performance (or at least the benchmark). I'll close this PR as unnecessary ceremony.

[lenguyenthanh]

hey, thanks anyway! We at least can cherry pick your removing imports commit.

I'm messing around with BinaryFen benchmark as it seems too simple (only one case) and compiler may constant folds it.

Edit: as expected, AbstractIterator can be used without affecting performance (or at least the benchmark). I'll close this PR as unnecessary ceremony.

Do you suggest to replace Iterator with AbstractIterator, as you mention in scala/scala3#22507?

[lenguyenthanh]

new benchmark code #620 and still nothing significant.