Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[add] added jmh microbenchmark, changed addDocuments to make usage of… #75

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

filipecosta90
Copy link

[add] added jmh microbenchmark for document ingestion
[upg] bumped jedis version from 3.01 to 3.1.0 ( to access sendCommand on Pipeline)
[add] changed addDocuments to make usage of pipelining ( see Client_addDocuments vs Client_addDocument microbenchmark ), leading to 16.4K OPS with Client_addDocument vs 18.3K OPS with Client_addDocuments


Client_addDocuments vs Client_addDocument microbenchmark

OSS redis-server + RediSearch ( current master ) runned with the following configs

redis-server --protected-mode no --save "" --appendonly no --loadmodule ./redisearch.so

Benchmark runned via:

$ mvn clean package
java -jar target/jredisearch-1.1.0-SNAPSHOT-perf-tests.jar -wi 0 -i 1 -t 16 -f 1 -r 30

output:

(...)
Benchmark                                        Mode  Cnt      Score   Error  Units
DocumentIngestionBenchmark.Client_addDocument   thrpt       16400.254          ops/s
DocumentIngestionBenchmark.Client_addDocuments  thrpt       18299.231          ops/s

… pipelining, bumped jedis version from 3.01 to 3.1.0
@codecov
Copy link

codecov bot commented Sep 16, 2019

Codecov Report

Merging #75 into master will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master      #75   +/-   ##
=======================================
  Coverage   80.93%   80.93%           
=======================================
  Files          34       34           
  Lines        1385     1385           
  Branches      203      202    -1     
=======================================
  Hits         1121     1121           
  Misses        190      190           
  Partials       74       74
Impacted Files Coverage Δ
src/main/java/io/redisearch/client/Client.java 84.75% <100%> (+0.27%) ⬆️
src/main/java/io/redisearch/Schema.java 90.36% <0%> (-0.65%) ⬇️
.../main/java/io/redisearch/client/ClusterClient.java 0% <0%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8159431...5876a35. Read the comment docs.

@@ -149,7 +223,7 @@
<configuration>
<aggregate>true</aggregate>
<additionalparam>-Xdoclint:none</additionalparam>
<additionalOptions>-Xdoclint:none</additionalOptions>
<!-- <additionalOptions>-Xdoclint:none</additionalOptions>-->
Copy link
Contributor

Choose a reason for hiding this comment

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

@filipecosta90 you can just remove this duplication

@gkorland
Copy link
Contributor

@filipecosta90 notice the build issue

.forks(5)
.build();

new Runner(opt).run();
Copy link
Contributor

Choose a reason for hiding this comment

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

is it a deamon thread? do you need to "join" on it?

@gkorland gkorland requested a review from sazzad16 April 11, 2021 10:26
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