Skip to content

Optimize memory usage#3874

Merged
NGDAdmin merged 55 commits intomasterfrom
mem-fix-usage
Apr 22, 2025
Merged

Optimize memory usage#3874
NGDAdmin merged 55 commits intomasterfrom
mem-fix-usage

Conversation

@shargon
Copy link
Member

@shargon shargon commented Apr 4, 2025

Description

The main problem is that we have two HashSetCache per remote node connected, every of them use 20_000 entries * 10 buckets, that's means 200_000 entries * 2 = 400_000 entries of 256 bytes, this can reach more 100mb of ram per node connected. I reduce this value from 400_000 entries to 20_000 per node.

Require #3904

Type of change

  • Optimization (the change is only an optimization)
  • Style (the change is only a code style for better maintenance or standard purpose)
  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

How Has This Been Tested?

  • Manual testing

Test Configuration:

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • Any dependent changes have been merged and published in downstream modules

@shargon shargon added the Bug Used to tag confirmed bugs label Apr 4, 2025
@shargon shargon added the Optimization Optmization issues label Apr 4, 2025
@Jim8y Jim8y requested a review from Copilot April 4, 2025 12:11
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.

Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.

Comments suppressed due to low confidence (1)

src/Neo/IO/Caching/HashSetCache.cs:97

  • The initializer '[]' is not valid syntax for instantiating a List in C#. Please replace it with 'new List<HashSet>()'.
List<HashSet<T>> removeList = [];

@shargon shargon added the Critical Issues (bugs) that need to be fixed ASAP label Apr 4, 2025
@shargon shargon changed the title Optimice mem usage Optimize mem usage Apr 4, 2025
@shargon shargon changed the title Optimize mem usage Optimize memory usage Apr 4, 2025
@shargon
Copy link
Member Author

shargon commented Apr 7, 2025

@neo-project/core ready to review, now it's configurable, and not related to the mempool variable

@shargon shargon requested a review from Jim8y April 8, 2025 14:25
@shargon shargon mentioned this pull request Apr 17, 2025
15 tasks
@shargon
Copy link
Member Author

shargon commented Apr 17, 2025

Require #3904

@shargon shargon mentioned this pull request Apr 17, 2025
15 tasks
@shargon shargon added Blocked This issue can't be worked at the moment and removed Need Update labels Apr 17, 2025
@shargon
Copy link
Member Author

shargon commented Apr 17, 2025

@neo-project/core Workflow fixed, we can review it and test it now

@NGDAdmin NGDAdmin merged commit 96e670d into master Apr 22, 2025
7 checks passed
@NGDAdmin NGDAdmin deleted the mem-fix-usage branch April 22, 2025 04:12
cschuchardt88 added a commit to cschuchardt88/neo that referenced this pull request Jun 8, 2025
* Fix mem usage

* Add clear when node disconnect

* fix count

* Add ut for clear

* Add doc an limit also TaskManager

* Same as mempool

* max

* min 100

* Move value to configuration

* divide by bucket

* use min 1

* Update src/Neo/Network/P2P/RemoteNode.ProtocolHandler.cs

* commented out debug.asserted

* Revert "commented out debug.asserted"

This reverts commit 052841c.

* revert to `TargetFrameworks`

* Merge branch 'mem-fix-usage' of github.com:neo-project/neo into mem-fix-usage

# Conflicts:
#	src/Neo/Ledger/Blockchain.cs

* merge

* merge

* Update src/Neo/Network/P2P/ChannelsConfig.cs

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

* Update src/Neo/Ledger/Blockchain.cs

* Update Blockchain.cs

* Update src/Neo/Ledger/Blockchain.cs

* Apply suggestions from code review

* Update Settings.cs

* Update MainService.cs

* for see logs

* Update main.yml

* Update .github/workflows/main.yml

* Update main.yml

* Update main.yml

* Update main.yml

* Remove parallel

* Update main.yml

* Update main.yml

* Update main.yml

---------

Co-authored-by: Will <201105916+Wi1l-B0t@users.noreply.github.com>
Co-authored-by: Jimmy <jinghui@wayne.edu>
Co-authored-by: Christopher Schuchardt <cschuchardt88@gmail.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Blocked This issue can't be worked at the moment Bug Used to tag confirmed bugs Critical Issues (bugs) that need to be fixed ASAP Optimization Optmization issues Ready to Merge

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants