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

chore: Implement AddMany method #3866

Merged
merged 2 commits into from
Oct 4, 2024
Merged

chore: Implement AddMany method #3866

merged 2 commits into from
Oct 4, 2024

Conversation

romange
Copy link
Collaborator

@romange romange commented Oct 4, 2024

  1. Fix a performance bug in Find2 that made redundant comparisons
  2. Provide a method to StringSet that adds several items in a batch
  3. Use AddMany inside set_family

Before:

BM_Add        4253939 ns      4253713 ns          991

After:

BM_Add        3482177 ns      3482050 ns         1206
BM_AddMany    3101622 ns      3101507 ns         1360

@romange
chore: Implement AddMany method
28b5d44 
1. Fix a performance bug in Find2 that made redundant comparisons
2. Provide a method to StringSet that adds several items in a batch
3. Use AddMany inside set_family

Before:
```
BM_Add        4253939 ns      4253713 ns          991
```

After:
```
BM_Add        3482177 ns      3482050 ns         1206
BM_AddMany    3101622 ns      3101507 ns         1360
```

Signed-off-by: Roman Gershman <roman@dragonflydb.io>
@romange romange requested a review from kostasrim October 4, 2024 14:27
@kostasrim
Copy link
Contributor

@romange

2024-10-04T14:44:23.4115778Z 60: F20241004 14:44:23.410632 22120 init.cc:25] [../src/core/dense_set.h:345]: assert(capacity_log_ > 0) failed!
2024-10-04T14:44:23.4117519Z 60: *** Check failure stack trace: ***
2024-10-04T14:44:23.4154314Z 60:     @          0x116db03  google::LogMessage::Fail()
2024-10-04T14:44:23.4184591Z 60:     @          0x116da4a  google::LogMessage::SendToLog()
2024-10-04T14:44:23.4193535Z 60: F20241004 14:44:23.418989 22119 init.cc:25] [../src/core/dense_set.h:345]: assert(capacity_log_ > 0) failed!
2024-10-04T14:44:23.4194758Z 60: *** Check failure stack trace: ***
2024-10-04T14:44:23.4225910Z 60:     @          0x116db03  google::LogMessage::Fail()
2024-10-04T14:44:23.4242747Z 60:     @          0x116d23f  google::LogMessage::Flush()
2024-10-04T14:44:23.4252989Z 60:     @          0x116da4a  google::LogMessage::SendToLog()
2024-10-04T14:44:23.4292718Z 60:     @          0x1170f56  google::LogMessageFatal::~LogMessageFatal()
2024-10-04T14:44:23.4310424Z 60:     @          0x116d23f  google::LogMessage::Flush()
2024-10-04T14:44:23.4322775Z 60:     @          0x116474d  __assert_fail
2024-10-04T14:44:23.4358823Z 60:     @           0x77537c  dfly::DenseSet::BucketId()
2024-10-04T14:44:23.4361239Z 60:     @          0x1170f56  google::LogMessageFatal::~LogMessageFatal()
2024-10-04T14:44:23.4389632Z 60:     @          0x116474d  __assert_fail
2024-10-04T14:44:23.4393117Z 60:     @           0xcfef51  dfly::DenseSet::Prefetch()
2024-10-04T14:44:23.4424961Z 60:     @           0x77537c  dfly::DenseSet::BucketId()
2024-10-04T14:44:23.4454083Z 60:     @           0xd04e51  dfly::StringSet::AddMany()
2024-10-04T14:44:23.4460588Z 60:     @           0xcfef51  dfly::DenseSet::Prefetch()
2024-10-04T14:44:23.4461702Z 60:     @           0x75cbf9  dfly::(anonymous namespace)::StringSetWrapper::Add()
2024-10-04T14:44:23.4467360Z 60:     @           0x75fd58  dfly::(anonymous namespace)::OpAdd()

kostasrim
kostasrim reviewed Oct 4, 2024
View reviewed changes
src/core/string_set.cc Outdated Show resolved Hide resolved
src/server/set_family.cc Outdated Show resolved Hide resolved
@romange romange requested a review from kostasrim October 4, 2024 16:11
kostasrim
kostasrim approved these changes Oct 4, 2024
View reviewed changes
src/core/string_set.cc
@@ -51,6 +51,42 @@ bool StringSet::Add(string_view src, uint32_t ttl_sec) {
return true;
}

unsigned StringSet::AddMany(absl::Span<std::string_view> span, uint32_t ttl_sec) {
Copy link
Contributor

Choose a reason for hiding this comment

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

So much simpler 😍

@romange romange merged commit bd972b6 into main Oct 4, 2024
12 checks passed
@romange romange deleted the Pr2 branch October 4, 2024 19:50
kostasrim pushed a commit that referenced this pull request Oct 7, 2024
@romange @kostasrim
chore: Implement AddMany method (#3866)
6970b66 
* chore: Implement AddMany method

1. Fix a performance bug in Find2 that made redundant comparisons
2. Provide a method to StringSet that adds several items in a batch
3. Use AddMany inside set_family

Before:
```
BM_Add        4253939 ns      4253713 ns          991
```

After:
```
BM_Add        3482177 ns      3482050 ns         1206
BM_AddMany    3101622 ns      3101507 ns         1360
```

Signed-off-by: Roman Gershman <roman@dragonflydb.io>

* chore: fixes

---------

Signed-off-by: Roman Gershman <roman@dragonflydb.io>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kostasrim kostasrim kostasrim approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@romange @kostasrim