migrate all vectorbench methods to lucene #12667
Conversation
|
You can still tell what's happening too due to the log messages. When each benchmark runs, you see a single message: xxxScalar() methods: xxxVector() methods: |
|
I have not yet checked the benchmakr fraework at all (earlier PR), but the idea here is cool:
so it runs with different comd line automatically by just the JMH annotation. yipee.! |
|
This looks great. I'll checkout the branch and play a little with, then report back. |
uschindler
left a comment
uschindler
left a comment
There was a problem hiding this comment.
Only the cleanup of documentation as suggested by Dawid would be good. Otherwise looks fine.
Have not played around, will report on windows with my laptop.
@dweiss is it enough to have RUNTIME_JAVA_HOME set when using Gradle to run benchmark with also preview versions not yet supported by Gradle?
|
I also removed some of the hacky self-tests the benchmarks were doing (comparing results across different impls). The reason is: when experimenting, you can easily validate the correctness with |
ChrisHegarty
left a comment
ChrisHegarty
left a comment
There was a problem hiding this comment.
This is awesome! And will really help with future maintenance in this area. Thank you.
|
I updated the docs, please have another look. |
|
Hi Robert, It is no longer needed, as the benchmark classes only call public Lucene methods. |
I noticed you need to run it on your own. We could mabye add some runner task to gradle, like |
I don't see this in my branch... |
It is no longer on main, too. Remove the "failOnMissingClasses = false" a few lines above instead. Dawid changed it later. |
|
Sorry for the chaotic comments, the I checked it with roberts branch: |
|
I just committed the change. |
I mentioned this on the original issue - I don't think forking a JMH subprocess from within gradle makes much sense as it may affect the results... and realistically, you'll want to test it out with multiple JVMs in parallel, I don't see much benefit here. |
uschindler
left a comment
uschindler
left a comment
There was a problem hiding this comment.
I found a bug! If I run the benchmark it always prints that vector module is used.
The reason for that is that in module descriptor it "requires jdk.incubator.vector". Therefor it enables it automaticlly in module mode (when you run with command line printed after "gradlew :...:assemble").
The jdk.incubator.vector must be removed from the module-info.java file.
|
Let me fix... |
…ys enables vectors. With that change it works.
|
Fixed. If you start the benchamrk now in module mode with the command line provided by "assemble" it works correct: |
The code will compile with the provided RUNTIME_JAVA_HOME but I intentionally left out the "launcher" - leaving just instructions on the console. If somebody uses JMH, they can point at whatever JVM they wish. I think using gradle to launch these measurement subprocesses is too crude. |
This was intentional, otherwise the example benchmark wouldn't compile... But since the vectors-specific code is removed now, I think removing this dependency is ok as well. |
The problem was: if you run the benchmark in module mode, the requirement added the module without respecting if the extra runtime parameter was given. I committed the change. It works now, benchmark is running. |
|
Yeah, i think its better not to compile benchmarks with incubator/preview etc? I think benchmarks should just call lucene methods, and lucene code should do the right thing with handling incubator/preview? |
|
I ran all benchmarks in module mode (second line of assemble output) on my AVX-256 laptop: Prozessor: Intel(R) Core(TM) i7-8565U CPU @ 1.80GHz, 1992 MHz, 4 Kern(e), 8 logische(r) Prozessor(en) |
If we backport this PR it would fail to compile and Jenkins would be unhappy, too. |
Yep, I understand. Did it also happen when it was started with -jar ("classpath" mode)? I don't think it should, right?
Hey, I just took Rob's example, that's it - had to add that module to compile it! :) |
Naaah, that was the most confusing part. |
|
The logging works correctly for my use-case (accidentally running java 17). Scalar method has no warnings, Vector method prints: |
* migrate all vectorbench methods to lucene Co-authored-by: Uwe Schindler <[email protected]>
4 participants
Following up to @dweiss work, this gives us the same benchmarks as https://github.com/rmuir/vectorbench, just without the code duplication and maintenance hassle.
Each method is simply invoked with and without
--add-modules=jdk.incubator.vector, so we can easily compare the performance of both the scalar and vector implementations.Same results we were getting before, with less code. I also like that it tests the "real" codepath through lucene, we just call VectorUtil methods with different JVM args.