Skip to content
New issue

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

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

Already on GitHub? # to your account

[♻️Housekeeping] Remove BufferPool class and use .net implementation #2508

Merged
merged 11 commits into from
Jun 19, 2024

Conversation

pictos
Copy link
Member

@pictos pictos commented Jun 12, 2024

There're a lot of places where Litedb news up an array just to discard it later, this is bad for allocations and perf. So:

  • I added the the ArrayPool provided by the .NET team
  • Removed old Pool implementations and replace then by .NET's one
  • Change, where is possible, in the code places that news up an array to discard then later to use the ArrayPool

Copy link
Collaborator

@JKamsker JKamsker left a comment

Choose a reason for hiding this comment

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

Generally, i would love to see this step to finally happen.

  • Why do we not centralize the buffer pool declaration or just use ArrayPool directly
  • Is the additional dependency also required in newer versions? Will it be installed in net8? The ArrayPool in net8 is in System.Runtime

LiteDB/Engine/Disk/Serializer/BufferReader.cs Outdated Show resolved Hide resolved
LiteDB/Engine/Disk/Serializer/BufferWriter.cs Outdated Show resolved Hide resolved
LiteDB/Engine/Disk/Streams/AesStream.cs Outdated Show resolved Hide resolved
LiteDB/Engine/Engine/Upgrade.cs Outdated Show resolved Hide resolved
LiteDB/Engine/FileReader/FileReaderV7.cs Show resolved Hide resolved
LiteDB/Engine/Sort/SortService.cs Outdated Show resolved Hide resolved
LiteDB/Engine/Sort/SortContainer.cs Outdated Show resolved Hide resolved
LiteDB/Engine/Structures/IndexNode.cs Show resolved Hide resolved
LiteDB/Engine/Disk/Serializer/BufferReader.cs Show resolved Hide resolved
LiteDB/Engine/Disk/Serializer/BufferReader.cs Outdated Show resolved Hide resolved
LiteDB/Engine/Disk/Serializer/BufferReader.cs Outdated Show resolved Hide resolved
LiteDB/Engine/Disk/Streams/AesStream.cs Outdated Show resolved Hide resolved
LiteDB/Engine/Engine/Upgrade.cs Outdated Show resolved Hide resolved
@pictos pictos requested a review from JKamsker June 14, 2024 02:29
Copy link
Collaborator

@mbdavid mbdavid left a comment

Choose a reason for hiding this comment

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

well done, thanks

@pictos pictos dismissed JKamsker’s stale review June 19, 2024 17:55

resolved comments

@pictos pictos merged commit b82f30e into master Jun 19, 2024
1 check passed
@pictos pictos deleted the pj/ArrayPool-4tw branch June 19, 2024 17:56
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants