fix(bitops): Fetch only single byte for SETBIT operation by mkaruza · Pull Request #6745 · dragonflydb/dragonfly

mkaruza · 2026-02-26T14:29:51Z

Instead of retrieving complete object we can get single byte on which
SETBIT operates.

augmentcode

Review completed. 5 suggestions posted.

Comment augment review to trigger a new review at any time.

src/core/compact_object.cc

src/core/detail/bitpacking.cc

src/server/bitops_family.cc

src/core/compact_object.h

src/core/compact_object.cc

Copilot

Pull request overview

This PR optimizes the SETBIT command to avoid reading and writing the entire string value when setting a single bit. Instead, it reads and writes only the affected byte when possible.

Changes:

Added byte-level access functions for ASCII-packed strings (ascii_pack_byte, ascii_unpack_byte)
Implemented GetByteAtIndex and SetByteAtIndex methods in CompactObj for efficient byte-level operations
Modified BitNewValue to use byte-level operations when the bit offset is within the existing string size

Reviewed changes

Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
src/core/detail/bitpacking.h	Added declarations for `ascii_pack_byte` and `ascii_unpack_byte` functions
src/core/detail/bitpacking.cc	Implemented byte-level pack/unpack for ASCII-encoded strings
src/core/compact_object.h	Added `GetByteAtIndex`, `SetByteAtIndex` methods and `DecodeByte` in StrEncoding
src/core/compact_object.cc	Implemented byte-level access for all CompactObj string encodings
src/core/compact_object_test.cc	Added comprehensive tests for byte-level operations (AsciiPackByte, GetByteAtOffset, SetByteAtOffset)
src/server/bitops_family.cc	Modified BitNewValue to use byte-level operations; added ElementAccess wrappers

src/server/bitops_family.cc

Copilot

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.

src/server/bitops_family.cc

romange

Please fix the inconsistencies.

Instead of retrieving complete object we can get single byte on which SETBIT operates. Signed-off-by: mkaruza <mario@dragonflydb.io>

Copilot

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 3 comments.

Copilot · 2026-03-01T11:01:08Z

src/server/bitops_family.cc

+  if (!success) {
+    updater_.post_updater.Run();
+  }


SetByteAtIndex runs post_updater.Run() only when success is false. Since success == false means the byte was not updated (typically out-of-bounds), running the post-updater here can incorrectly mark the key as updated without any actual mutation. Consider treating success == false as a programming error (e.g. DCHECK/LOG(DFATAL)) or handling the failure explicitly (e.g. extend+commit) instead of calling post_updater.Run().

Suggested change

if (!success) {

updater_.post_updater.Run();

}

DCHECK(success);

src/server/bitops_family.cc

Claude · 2026-03-01T11:26:38Z

@romange I've opened a new pull request, #6770, to work on those changes. Once the pull request is ready, I'll request review from you.

* Initial plan * test(bitops): Add test for SETBIT extending existing key Addresses feedback from code review to add test coverage for the scenario where SETBIT extends an existing key beyond its current length. Co-authored-by: romange <3674760+romange@users.noreply.github.com> --------- Co-authored-by: anthropic-code-agent[bot] <242468646+Claude@users.noreply.github.com> Co-authored-by: romange <3674760+romange@users.noreply.github.com>

mkaruza · 2026-03-01T14:34:36Z

@copilot please remove ElementAccess::Size() function because it's not used anymore

Copilot · 2026-03-01T14:34:43Z

@mkaruza I've opened a new pull request, #6773, to work on those changes. Once the pull request is ready, I'll request review from you.

* Initial plan * Remove unused ElementAccess::Size() method Co-authored-by: mkaruza <3676457+mkaruza@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: mkaruza <3676457+mkaruza@users.noreply.github.com>

Copilot

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

src/server/bitops_family.cc

Copilot

Pull request overview

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

Optimizes the SETBIT command to avoid reading and writing the entire string value when setting a single bit. Instead, it reads and writes only the affected byte when possible. Signed-off-by: mkaruza <mario@dragonflydb.io>

mkaruza requested review from Copilot, kostasrim and romange and removed request for Copilot February 26, 2026 14:29

augmentcode bot reviewed Feb 26, 2026

View reviewed changes

romange reviewed Feb 26, 2026

View reviewed changes

src/core/compact_object.h Outdated Show resolved Hide resolved

romange reviewed Feb 26, 2026

View reviewed changes

src/core/compact_object.cc Outdated Show resolved Hide resolved

romange reviewed Feb 26, 2026

View reviewed changes

src/core/compact_object.cc Outdated Show resolved Hide resolved

romange reviewed Feb 26, 2026

View reviewed changes

src/core/compact_object.cc Outdated Show resolved Hide resolved

romange reviewed Feb 26, 2026

View reviewed changes

src/core/compact_object.cc Outdated Show resolved Hide resolved

romange reviewed Feb 26, 2026

View reviewed changes

src/core/compact_object.cc Outdated Show resolved Hide resolved

Copilot AI review requested due to automatic review settings February 27, 2026 12:56

mkaruza force-pushed the mkaruza/bitops-fix branch from 6027966 to 9c2612a Compare February 27, 2026 12:56

Copilot started reviewing on behalf of mkaruza February 27, 2026 12:56 View session

mkaruza changed the base branch from main to mkaruza/get-set-byte-compact-object February 27, 2026 12:57

mkaruza changed the title ~~fix(bitops): Read only one byte with SETBIT~~ fix(bitops): Fetch only single byte for SETBIT operation Feb 27, 2026

Copilot AI reviewed Feb 27, 2026

View reviewed changes

src/server/bitops_family.cc Outdated Show resolved Hide resolved

mkaruza force-pushed the mkaruza/bitops-fix branch 2 times, most recently from f187fb1 to cb9a95c Compare February 27, 2026 14:47

mkaruza force-pushed the mkaruza/get-set-byte-compact-object branch from f6401ef to 4dbb27f Compare February 27, 2026 21:08

Base automatically changed from mkaruza/get-set-byte-compact-object to main February 28, 2026 20:26

mkaruza force-pushed the mkaruza/bitops-fix branch from bd65fbf to c489984 Compare February 28, 2026 20:32

Copilot AI review requested due to automatic review settings February 28, 2026 20:32

Copilot started reviewing on behalf of mkaruza February 28, 2026 20:32 View session

Copilot AI reviewed Feb 28, 2026

View reviewed changes

src/server/bitops_family.cc Outdated Show resolved Hide resolved

mkaruza requested a review from romange February 28, 2026 21:15

romange reviewed Mar 1, 2026

View reviewed changes

src/server/bitops_family.cc Outdated Show resolved Hide resolved

romange requested changes Mar 1, 2026

View reviewed changes

mkaruza added 2 commits March 1, 2026 10:53

fix(bitops): Fetch only single byte for SETBIT operation

1b57b24

Instead of retrieving complete object we can get single byte on which SETBIT operates. Signed-off-by: mkaruza <mario@dragonflydb.io>

Always commit extended key

8ea29c2

Use GetByteAtIndex to check if offset is oob

0429336

Copilot AI review requested due to automatic review settings March 1, 2026 10:54

mkaruza force-pushed the mkaruza/bitops-fix branch from e6ea53a to 0429336 Compare March 1, 2026 10:54

Copilot started reviewing on behalf of mkaruza March 1, 2026 10:54 View session

Copilot AI reviewed Mar 1, 2026

View reviewed changes

Claude AI mentioned this pull request Mar 1, 2026

test(bitops): Add test for SETBIT extending existing key #6770

Merged

Copilot AI mentioned this pull request Mar 1, 2026

Remove unused ElementAccess::Size() method #6773

Merged

Copilot AI review requested due to automatic review settings March 1, 2026 14:49

Copilot started reviewing on behalf of mkaruza March 1, 2026 14:49 View session

Copilot AI reviewed Mar 1, 2026

View reviewed changes

src/server/bitops_family.cc Outdated Show resolved Hide resolved

Check correct result from SetByteAtIndex

9d4664b

romange previously approved these changes Mar 1, 2026

View reviewed changes

romange reviewed Mar 1, 2026

View reviewed changes

src/server/bitops_family.cc Outdated Show resolved Hide resolved

romange reviewed Mar 1, 2026

View reviewed changes

src/server/bitops_family.cc Outdated Show resolved Hide resolved

Final PR review changes

9299ec7

Copilot AI review requested due to automatic review settings March 1, 2026 18:32

mkaruza dismissed romange’s stale review via 9299ec7 March 1, 2026 18:32

Copilot started reviewing on behalf of mkaruza March 1, 2026 18:32 View session

Copilot AI reviewed Mar 1, 2026

View reviewed changes

romange approved these changes Mar 1, 2026

View reviewed changes

mkaruza merged commit 9c5ab55 into main Mar 1, 2026
16 checks passed

mkaruza deleted the mkaruza/bitops-fix branch March 1, 2026 19:43

miso-assistant bot mentioned this pull request Mar 26, 2026

fix(container): update image ghcr.io/dragonflydb/dragonfly (v1.37.0 → v1.37.2) joryirving/home-ops#6498

Merged

1 task

Conversation

mkaruza commented Feb 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

augmentcode bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

romange left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Mar 1, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Claude AI commented Mar 1, 2026

Uh oh!

mkaruza commented Mar 1, 2026

Uh oh!

Copilot AI commented Mar 1, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

mkaruza commented Feb 26, 2026 •

edited

Loading