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

Additional improvements to memory pooling and management. #2960

Merged
merged 4 commits into from
Mar 29, 2022
Merged

Conversation

derekcollison
Copy link
Member

During contention to the head write block, the system could perform worse memory wise compared to simple go runtime and GC. Also had some references for the subject of messages being read back in to the underlying block which was bloating memory.

Additionally a fix for firstMatching() that did unnecessary work when matching all. This would cause excessive CPU and excessive load on the garbage collector.

Signed-off-by: Derek Collison derek@nats.io

/cc @nats-io/core

@@ -2047,6 +2069,7 @@ func (fs *fileStore) removeMsg(seq uint64, secure, needFSLock bool) (bool, error
// we don't lose track of the first sequence.
if firstSeqNeedsUpdate {
fs.selectNextFirst()
// FIXME(dlc)?
Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks for reminder! I don't think that statement is correct, will look more closely now.

}
}
}
// Copy here to not reference underlying buffer.
Copy link
Member

Choose a reason for hiding this comment

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

I don't understand this... subjString() is invoked with a string that is already a string (either from caller stack or made a string out of the underlying buffer), so I don't see how "key" is referencing the underlying buffer.

Copy link
Member Author

Choose a reason for hiding this comment

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

I was seeing higher memory usage until this change, if it was referencing the underlying buffer I believe runtime could reference it vs copy on the cast.

…fix for firstMatching that did unnecessary work when matching all.

During contention to the head write blk, the system could perform worse memory wise compared to simple go runtime.
Also had some references for the subject of messages bloating memory.

Signed-off-by: Derek Collison <derek@nats.io>
kozlovic and others added 3 commits March 28, 2022 17:37
This may prevent memory copies when not necessary. Also fixed a bug
there that would check twice if there was only 1 subject and that
subject did not match (say configured subject is foo.* and key is
foo.bar).

Signed-off-by: Ivan Kozlovic <ivan@synadia.com>
Signed-off-by: Ivan Kozlovic <ivan@synadia.com>
…. This would possibly cause the load code to thrash since it would not be big enough for a full block and we would need to recycle again and make a new one.

Signed-off-by: Derek Collison <derek@nats.io>
Copy link
Member

@kozlovic kozlovic left a comment

Choose a reason for hiding this comment

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

LGTM

@derekcollison derekcollison merged commit 780d4c0 into main Mar 29, 2022
@derekcollison derekcollison deleted the mem_pool branch March 29, 2022 00:10
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.

None yet

2 participants