Skip to content

Comments

perf(example): add PerfExample5 and PerfExample6#2860

Merged
VGalaxies merged 2 commits intomasterfrom
perf-example6
Sep 16, 2025
Merged

perf(example): add PerfExample5 and PerfExample6#2860
VGalaxies merged 2 commits intomasterfrom
perf-example6

Conversation

@javeme
Copy link
Contributor

@javeme javeme commented Aug 30, 2025

Change-Id: Ibe3df29c6926c5c58bab217220f3005d92476f7b

Change-Id: Ibe3df29c6926c5c58bab217220f3005d92476f7b
@dosubot dosubot bot added size:L This PR changes 100-499 lines, ignoring generated files. perf labels Aug 30, 2025
@codecov
Copy link

codecov bot commented Aug 30, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 42.35%. Comparing base (0946d5d) to head (6491e65).
⚠️ Report is 3 commits behind head on master.

Additional details and impacted files
@@             Coverage Diff              @@
##             master    #2860      +/-   ##
============================================
+ Coverage     40.91%   42.35%   +1.44%     
- Complexity      333      561     +228     
============================================
  Files           747      756       +9     
  Lines         60168    60435     +267     
  Branches       7683     7705      +22     
============================================
+ Hits          24615    25599     +984     
+ Misses        32975    32192     -783     
- Partials       2578     2644      +66     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@javeme javeme requested review from imbajin and z7658329 August 30, 2025 16:57
*/
public class PerfExample1 extends PerfExampleBase {

private static final Logger LOG = Log.logger(PerfExample1.class);
Copy link
Member

Choose a reason for hiding this comment

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

The logger declaration in PerfExample1 and PerfExample2 is added but never used. Consider removing these unused imports and logger declarations to avoid dead code.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

this time we marked the PerfExampleBase.LOG as private.

*/
public class PerfExample2 extends PerfExampleBase {

private static final Logger LOG = Log.logger(PerfExample2.class);
Copy link
Member

Choose a reason for hiding this comment

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

Same issue as PerfExample1 - logger is declared but never used. Remove the unused import and logger declaration.

import org.apache.tinkerpop.gremlin.structure.Edge;
import org.slf4j.Logger;

/**
Copy link
Member

Choose a reason for hiding this comment

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

The comment says 'query vertices/adj-edges by ids' but this class only provides vertex and edge querying functionality. The comment should be more specific about what this example demonstrates compared to PerfExample3.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

PerfExample3 is a test for: insert vertices with indexes in order


/**
* Perf test for: query vertices/adj-edges by ids
*/
Copy link
Member

Choose a reason for hiding this comment

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

The comment is identical to PerfExample5 but this class has different functionality (batch insertion with edges). Update the comment to accurately describe what PerfExample6 demonstrates.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In this PerfExample6 test, we insert some edges (one edge per vertex) for adj-edges test.


@Override
protected void testInsert(GraphManager graph, int times, int multiple) {
final int TIMES = times * multiple;
Copy link
Member

Choose a reason for hiding this comment

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

Consider extracting the BATCH constant to a class-level constant or making it configurable, as hardcoded values like 100 reduce flexibility for performance testing.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Only the insertion test involves batch, and batch is not a dimension we test, so keep it for now.

totalV, thread, totalE);
}

protected static long elapsed(long start) {
Copy link
Member

Choose a reason for hiding this comment

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

Good refactoring to move the elapsed() method to the base class where it can be reused by all performance examples.

@imbajin imbajin requested a review from Copilot September 1, 2025 08:28
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR introduces two new performance testing examples (PerfExample5 and PerfExample6) for HugeGraph and refactors existing performance testing code. The changes focus on improving query performance testing capabilities and code organization.

  • Adds PerfExample5 for testing vertex/edge queries by IDs with improved performance measurement
  • Adds PerfExample6 extending PerfExample5 with sequential vertex insertion and edge creation
  • Refactors the base class and existing examples to improve code reusability and logging consistency

Reviewed Changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
PerfExampleBase.java Refactors query methods, adds utility functions, and improves logging with runtime parameters
PerfExample5.java New performance test class for vertex/edge queries by IDs with detailed timing measurements
PerfExample6.java New performance test extending PerfExample5 with sequential data insertion patterns
PerfExample4.java Updates logger reference and removes duplicated elapsed() method
PerfExample1.java Adds missing logger import and declaration
PerfExample2.java Adds missing logger import and declaration

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

public static final int EDGE_NUM = 100;

protected static final Logger LOG = Log.logger(PerfExampleBase.class);
private static final Logger LOG = Log.logger(PerfExampleBase.class);
Copy link

Copilot AI Sep 1, 2025

Choose a reason for hiding this comment

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

Changing the Logger from 'protected' to 'private' breaks inheritance for subclasses that may need to access the logger. Consider keeping it protected or having subclasses define their own loggers.

Suggested change
private static final Logger LOG = Log.logger(PerfExampleBase.class);
protected static final Logger LOG = Log.logger(PerfExampleBase.class);

Copilot uses AI. Check for mistakes.
Comment on lines +214 to +215
LOG.info("Query {} vertices with thread({}): {} vertices/s",
totalV, thread, totalV * 1000 / cost);
Copy link

Copilot AI Sep 1, 2025

Choose a reason for hiding this comment

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

Division by zero error will occur if 'cost' is 0. Add a check to prevent division by zero before calculating vertices/s.

Suggested change
LOG.info("Query {} vertices with thread({}): {} vertices/s",
totalV, thread, totalV * 1000 / cost);
if (cost == 0) {
LOG.info("Query {} vertices with thread({}): N/A vertices/s (cost=0ms)", totalV, thread);
} else {
LOG.info("Query {} vertices with thread({}): {} vertices/s",
totalV, thread, totalV * 1000 / cost);
}

Copilot uses AI. Check for mistakes.
Comment on lines +67 to +68
LOG.info("Query {} vertices with thread({}): {} vertices/s",
totalV, thread, totalV * 1000 / cost);
Copy link

Copilot AI Sep 1, 2025

Choose a reason for hiding this comment

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

Division by zero error will occur if 'cost' is 0. Add a check to prevent division by zero before calculating vertices/s.

Suggested change
LOG.info("Query {} vertices with thread({}): {} vertices/s",
totalV, thread, totalV * 1000 / cost);
if (cost == 0) {
LOG.info("Query {} vertices with thread({}): N/A vertices/s (cost=0)", totalV, thread);
} else {
LOG.info("Query {} vertices with thread({}): {} vertices/s",
totalV, thread, totalV * 1000 / cost);
}

Copilot uses AI. Check for mistakes.
Comment on lines +96 to +99
LOG.info("Query {} edges of vertices({}) with thread({}): " +
"{} vertices/s, {} edges/s",
totalE, totalV, thread,
totalV * 1000 / cost, totalE * 1000 / cost);
Copy link

Copilot AI Sep 1, 2025

Choose a reason for hiding this comment

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

Division by zero error will occur if 'cost' is 0. Add a check to prevent division by zero before calculating vertices/s and edges/s.

Copilot uses AI. Check for mistakes.
Change-Id: Ifeeba9d0aefdb70624e334f425271cc1c48d3734
@dosubot dosubot bot added the lgtm This PR has been approved by a maintainer label Sep 12, 2025
@imbajin imbajin requested a review from VGalaxies September 12, 2025 05:54
@VGalaxies VGalaxies merged commit b642b5a into master Sep 16, 2025
14 of 15 checks passed
@VGalaxies VGalaxies deleted the perf-example6 branch September 16, 2025 02:25
Tsukilc pushed a commit to hugegraph/hugegraph that referenced this pull request Sep 23, 2025
Tsukilc pushed a commit to hugegraph/hugegraph that referenced this pull request Sep 23, 2025
Tsukilc pushed a commit to hugegraph/hugegraph that referenced this pull request Sep 23, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

lgtm This PR has been approved by a maintainer perf size:L This PR changes 100-499 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants