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.

Sign up for GitHub

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

Jump to bottom

Cache redesign #351

Merged
merged 1 commit into from
Mar 25, 2020
Merged

Cache redesign #351

merged 1 commit into from
Mar 25, 2020

Conversation

iceseer
Copy link
Contributor

@iceseer iceseer commented Feb 19, 2020

Description of the Change

Redesign cache with ring buffer. It shows much better performance.

Benefits

performance profit

Possible Drawbacks

Usage Examples or Tests [optional]

Alternate Designs [optional]

MBoldyrev
MBoldyrev reviewed Feb 19, 2020
View reviewed changes
Copy link
Contributor

@MBoldyrev MBoldyrev left a comment

Choose a reason for hiding this comment

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

Looks great, thanks! I like the new cache, but I think the ring buffer can be reworked with iterators pair instead of active in each element.

libs/cache/cache.hpp Outdated Show resolved Hide resolved
libs/common/ring_buffer.hpp Outdated Show resolved Hide resolved
libs/cache/cache.hpp Outdated Show resolved Hide resolved
libs/cache/cache.hpp Show resolved Hide resolved
libs/common/ring_buffer.hpp Outdated Show resolved Hide resolved
libs/common/ring_buffer.hpp Outdated Show resolved Hide resolved
libs/common/ring_buffer.hpp Outdated Show resolved Hide resolved
libs/common/ring_buffer.hpp Outdated Show resolved Hide resolved
libs/common/ring_buffer.hpp Outdated Show resolved Hide resolved
MBoldyrev
MBoldyrev reviewed Feb 21, 2020
View reviewed changes
Copy link
Contributor

@MBoldyrev MBoldyrev left a comment

Choose a reason for hiding this comment

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

the implementation of ring buffer is interesting, though takes some time to understand.

libs/common/ring_buffer.hpp Outdated Show resolved Hide resolved
libs/common/ring_buffer.hpp Outdated Show resolved Hide resolved
libs/common/ring_buffer.hpp Outdated Show resolved Hide resolved
libs/common/ring_buffer.hpp Outdated Show resolved Hide resolved
libs/common/ring_buffer.hpp Outdated Show resolved Hide resolved
@iceseer iceseer force-pushed the feature/cache branch 4 times, most recently from 99504ef to 07ed14c Compare February 24, 2020 13:38
@lebdron lebdron changed the title Cashe redesign Cache redesign Feb 28, 2020
@MBoldyrev MBoldyrev requested a review from lebdron March 4, 2020 12:35
@MBoldyrev MBoldyrev added the 1.x label Mar 12, 2020
lebdron
lebdron reviewed Mar 19, 2020
View reviewed changes
Copy link
Contributor

@lebdron lebdron left a comment

Choose a reason for hiding this comment

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

It seems to be OK in general, but please refactor names and casts to Google Codestyle.
https://google.github.io/styleguide/cppguide.html#Casting , and no trailing underscores.

libs/common/ring_buffer.hpp Outdated Show resolved Hide resolved
libs/cache/cache.hpp Outdated Show resolved Hide resolved
libs/cache/cache.hpp Outdated Show resolved Hide resolved
libs/common/ring_buffer.hpp Outdated Show resolved Hide resolved
libs/common/ring_buffer.hpp Outdated Show resolved Hide resolved
test/module/libs/cache/cache_test.cpp Outdated Show resolved Hide resolved
shared_model/cryptography/hash.hpp Outdated Show resolved Hide resolved
@iceseer iceseer requested a review from lebdron March 19, 2020 17:41
@lebdron lebdron removed the request for review from ekovalev March 20, 2020 08:53
lebdron
lebdron approved these changes Mar 23, 2020
View reviewed changes
Copy link
Contributor

@lebdron lebdron left a comment

Choose a reason for hiding this comment

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

Thanks! Please apply clang-format, fix DCO, and check the comments.

libs/common/ring_buffer.hpp Outdated Show resolved Hide resolved
libs/common/ring_buffer.hpp Outdated Show resolved Hide resolved
libs/common/ring_buffer.hpp Outdated Show resolved Hide resolved
libs/common/ring_buffer.hpp Outdated Show resolved Hide resolved
libs/common/ring_buffer.hpp Outdated Show resolved Hide resolved
libs/common/ring_buffer.hpp Outdated Show resolved Hide resolved
libs/common/ring_buffer.hpp Outdated Show resolved Hide resolved
libs/cache/cache.hpp Outdated Show resolved Hide resolved
libs/cache/cache.hpp Outdated Show resolved Hide resolved
libs/common/ring_buffer.hpp Outdated Show resolved Hide resolved
@iceseer iceseer closed this Mar 25, 2020
@iceseer
cache based on ring buffer
05526eb 
Signed-off-by: iceseer <iceseer@gmail.com>
@iceseer iceseer reopened this Mar 25, 2020
@iceseer iceseer merged commit 48e6cca into hyperledger-iroha:master Mar 25, 2020
@nxsaken nxsaken added the iroha1 The legacy version of Iroha. label Apr 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@MBoldyrev MBoldyrev MBoldyrev approved these changes

@lebdron lebdron lebdron approved these changes

Assignees
No one assigned
Labels
1.x iroha1 The legacy version of Iroha.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@iceseer @MBoldyrev @lebdron @nxsaken