Skip to content

Conversation

@DavIvek
Copy link
Contributor

@DavIvek DavIvek commented Mar 31, 2025

Description

This PR aims to improve performance of the betweenness centrality implementation.

Pull request type

  • Bugfix
  • Algorithm/Module
  • Feature
  • Code style update (formatting, renaming)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • Documentation content changes
  • Other (please describe):

Related issues

Delete if this PR doesn't resolve any issues. Link the issue if it does.

######################################

Reviewer checklist (the reviewer checks this part)

Module/Algorithm

  • Core algorithm/module implementation
  • Query module implementation
  • Tests provided (unit / e2e)
  • Code documentation
  • README short description

Documentation checklist

  • Add the documentation label tag
  • Add the bug / feature label tag
  • Add the milestone for which this feature is intended
    • If not known, set for a later milestone
  • Write a release note, including added/changed clauses
    • The performance of the betweenness centrality and online betweenness centrality algorithms has been improved, with speedups exceeding 100% in some cases. By default, the algorithm now uses half of the available hardware threads for parallelization. #588
  • Link the documentation PR here
  • Tag someone from docs team in the comments
    @katarinasupe

@DavIvek DavIvek added Docs - changelog only Docs - changelog only enhancement enhancement labels Mar 31, 2025
@DavIvek DavIvek added this to the mage-v3.2.0 milestone Mar 31, 2025
@DavIvek DavIvek self-assigned this Mar 31, 2025
@DavIvek DavIvek force-pushed the betweenness-centrality-improvements branch from 27a36c7 to 210b794 Compare April 1, 2025 16:13
@DavIvek DavIvek marked this pull request as ready for review April 1, 2025 18:39
@DavIvek DavIvek requested a review from imilinovic April 1, 2025 18:39
Copy link
Contributor

@imilinovic imilinovic left a comment

Choose a reason for hiding this comment

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

Two small changes.

From what I see this still uses GraphView (possible improvement for future) and shouldn't change memory usage, only speed, is this correct?

@DavIvek
Copy link
Contributor Author

DavIvek commented Apr 1, 2025

@imilinovic Correct. This implementation still uses GraphView, and improving it in the future would be beneficial. However, making this change would require more modifications because the online version of betweenness centrality depends on this implementation. As far as I know, changing GraphView would require updates in both the Mage and Memgraph repositories since we have also moved that algorithm to the Memgraph repository. I didn't make this change in this PR because it is not the main pain point of this implementation, but it is definitely an improvement to consider for the next iteration.

@DavIvek DavIvek added Docs needed Docs needed and removed Docs - changelog only Docs - changelog only labels Apr 1, 2025
@sonarqubecloud
Copy link

sonarqubecloud bot commented Apr 1, 2025

@DavIvek DavIvek enabled auto-merge April 1, 2025 21:47
@DavIvek DavIvek added this pull request to the merge queue Apr 1, 2025
Merged via the queue into main with commit b36c116 Apr 1, 2025
10 checks passed
@DavIvek DavIvek deleted the betweenness-centrality-improvements branch April 1, 2025 22:57
@gitbuda gitbuda mentioned this pull request Apr 13, 2025
53 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Docs needed Docs needed enhancement enhancement

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants