feat(json): Adds a reference counted string class #6497

abhijat · 2026-01-29T06:31:23Z

Wraps the blob introduced in feat(json): Adds a ref-counted blob #6456
Handles reference count and lifetimes
Exposes a string api for jsoncons

The code is not hooked into jsoncons yet, it will be done in a small, final PR, kept separate because a lot of tests have to be adjusted at that point.

src/core/json/detail/interned_string.cc

augmentcode · 2026-01-29T08:26:19Z

🤖 Augment PR Summary

Summary: Introduces a new reference-counted string wrapper to support JSON key/value lifetimes on top of the existing interned-blob representation.

Changes:

Adds detail::InternedString, a RAII type that interns strings into a thread-local InternedBlobPool and manages blob refcounts on copy/move/destruction.
Extends InternedBlobHandle to better support empty values (empty handle, Size()=0, string_view conversion to "") and adds convenience operators.
Adds pool management helpers (InternedString::GetPoolRef(), ResetPool()) to control shutdown ordering vs. the thread-local memory resource.
Updates JSON build targets to compile the new implementation file.
Expands unit tests to validate interning behavior, refcounting, move/copy semantics, and the exposed string-like API expected by jsoncons.

Technical Notes: The PR explicitly does not wire InternedString into jsoncons yet; that integration (and associated test adjustments) is deferred to a follow-up PR.

_{🤖 Was this summary useful? React with 👍 or 👎}

augmentcode

Review completed. 2 suggestions posted.

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

src/core/json/detail/interned_blob.cc

src/core/json/detail/interned_string.h

Copilot

Pull request overview

This PR introduces InternedString, a reference-counted string class that wraps the InternedBlobHandle from PR #6456. The class manages string interning and reference counting for JSON keys, providing a memory-efficient way to store duplicate strings by maintaining a thread-local pool of unique string blobs. The implementation includes lifecycle management, string API compatibility with jsoncons, and comprehensive test coverage.

Changes:

Introduces InternedString class that manages reference counting and lifetimes for interned string blobs
Enhances InternedBlobHandle with comparison operators and explicit bool conversion for null checks
Removes the raw SetRefCount() method to enforce proper reference count management through the public API
Updates tests to use the new API and adds comprehensive tests for InternedString functionality

Reviewed changes

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

Show a summary per file

File	Description
src/core/json/detail/interned_string.h	Defines the InternedString class with RAII semantics for reference-counted string interning, including constructors required by jsoncons
src/core/json/detail/interned_string.cc	Implements reference counting logic (Acquire/Release), string interning with deduplication, and thread-local pool management
src/core/json/detail/interned_blob.h	Adds comparison operators and explicit bool conversion; removes SetRefCount to enforce encapsulation
src/core/json/detail/interned_blob.cc	Updates blob creation and accessors to handle empty strings/null pointers; removes SetRefCount implementation
src/core/json/interned_blob_test.cc	Updates existing tests to use IncrRefCount instead of removed SetRefCount; adds comprehensive tests for InternedString including pool behavior, string API, and constructor edge cases
src/core/json/CMakeLists.txt	Adds interned_string.cc to the build configuration

romange · 2026-01-29T10:18:45Z

src/core/json/detail/interned_blob.h

+  [[nodiscard]] static InternedBlobHandle Create(std::string_view sv);

-  uint32_t RefCount() const;
+  [[nodiscard]] uint32_t Size() const;


I do not see value in adding [[nodiscard]] to all the accessors. the only function that matters imho is Create as it can potentially cause a leak

romange · 2026-01-29T10:19:48Z

src/core/json/detail/interned_string.cc

+}
+
+int InternedString::compare(const InternedString& other) const {
+  return std::string_view{*this}.compare(other);


move one-liners to header files

romange · 2026-01-29T16:07:57Z

src/core/json/detail/interned_string.cc

@@ -0,0 +1,78 @@
+// Copyright 2025, DragonflyDB authors.  All rights reserved.


btw, please change the date to 2026 in all the touched files.

romange

lgtm

dranikpg

Seems fine from a design perspective

src/core/json/detail/interned_string.cc

abhijat · 2026-01-30T06:44:19Z

Merging is blocked
Commits must have verified signatures.

I missed signature in a commit. Will have to rebase

A null blob is now allowed. The reason is that sometimes jsoncons will create an empty string. We want to allow for empty strings without carrying the "" and its metadata around, and without adding "" to the pool. For this reason a null blob and associated handle is allowed. It provides short circuited accessors, but refcount on such a blob is a programming error. In addition comparison operators are added to be able to compare the InternedString objects trivially. Signed-off-by: Abhijat Malviya <[email protected]>

Signed-off-by: Abhijat Malviya <[email protected]>

The string class wraps the previously added blob class. It decides at creation time whether to create a new blob, or bump up the refcount of an existing blob in the pool. The pool is thread local, and is used by the strings on that thread to store pointers to allocated blobs. When a string is destroyed it decrements the ref-count, dropping the blob if necessary (ie refcount == 0). A lot of boilerplate in this class is from trial and error, adding methods that jsoncons required to be able to use this class as a key. Signed-off-by: Abhijat Malviya <[email protected]>

Signed-off-by: Abhijat Malviya <[email protected]>

Copilot

Pull request overview

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

romange · 2026-01-30T07:10:38Z

src/core/json/detail/interned_blob.cc

 InternedBlobHandle::operator std::string_view() const {
-  DCHECK(blob_) << "Attempt to convert empty blob to string_view";
-  return {blob_, Size()};
+  return blob_ ? std::string_view{blob_, Size()} : "";


i could be in header file.

abhijat force-pushed the abhijat/feat/ref-cnt-str branch 3 times, most recently from 1b99cf1 to 1b96eec Compare January 29, 2026 06:39

abhijat commented Jan 29, 2026

View reviewed changes

src/core/json/detail/interned_string.cc Outdated Show resolved Hide resolved

abhijat force-pushed the abhijat/feat/ref-cnt-str branch 6 times, most recently from 2f0dad8 to 3269461 Compare January 29, 2026 08:18

abhijat marked this pull request as ready for review January 29, 2026 08:19

Copilot AI review requested due to automatic review settings January 29, 2026 08:19

Copilot started reviewing on behalf of abhijat January 29, 2026 08:19 View session

abhijat force-pushed the abhijat/feat/ref-cnt-str branch from 3269461 to 9b3c115 Compare January 29, 2026 08:25

augmentcode bot reviewed Jan 29, 2026

View reviewed changes

src/core/json/detail/interned_blob.cc Show resolved Hide resolved

src/core/json/detail/interned_string.h Show resolved Hide resolved

abhijat force-pushed the abhijat/feat/ref-cnt-str branch from 9b3c115 to 7cd8f4d Compare January 29, 2026 08:27

Copilot AI reviewed Jan 29, 2026

View reviewed changes

abhijat requested review from dranikpg and romange January 29, 2026 09:49

abhijat mentioned this pull request Jan 29, 2026

feat(json): Adds a ref-counted blob #6456

Merged

romange reviewed Jan 29, 2026

View reviewed changes

abhijat requested a review from romange January 29, 2026 10:47

romange reviewed Jan 29, 2026

View reviewed changes

romange previously approved these changes Jan 29, 2026

View reviewed changes

dranikpg previously approved these changes Jan 29, 2026

View reviewed changes

src/core/json/detail/interned_string.cc Outdated Show resolved Hide resolved

abhijat enabled auto-merge (squash) January 30, 2026 06:43

abhijat added 5 commits January 30, 2026 12:14

json: Drop SetRefCount API, adjust tests

0d432e9

Signed-off-by: Abhijat Malviya <[email protected]>

address review comments

da66071

Signed-off-by: Abhijat Malviya <[email protected]>

update copyright

46bb1f0

Signed-off-by: Abhijat Malviya <[email protected]>

copy-and-swap

99cf817

Signed-off-by: Abhijat Malviya <[email protected]>

Copilot AI review requested due to automatic review settings January 30, 2026 06:54

abhijat dismissed stale reviews from dranikpg and romange via 99cf817 January 30, 2026 06:54

abhijat force-pushed the abhijat/feat/ref-cnt-str branch from 95793e5 to 99cf817 Compare January 30, 2026 06:54

Copilot started reviewing on behalf of abhijat January 30, 2026 06:54 View session

abhijat disabled auto-merge January 30, 2026 07:01

abhijat requested review from dranikpg and romange January 30, 2026 07:02

Copilot AI reviewed Jan 30, 2026

View reviewed changes

romange reviewed Jan 30, 2026

View reviewed changes

romange approved these changes Jan 30, 2026

View reviewed changes

abhijat enabled auto-merge (squash) January 30, 2026 07:15

abhijat merged commit 5ef19ff into main Jan 30, 2026
16 checks passed

abhijat deleted the abhijat/feat/ref-cnt-str branch January 30, 2026 08:07

		@@ -0,0 +1,78 @@
		// Copyright 2025, DragonflyDB authors. All rights reserved.

feat(json): Adds a reference counted string class #6497

feat(json): Adds a reference counted string class #6497

Conversation

abhijat commented Jan 29, 2026

Uh oh!

Uh oh!

augmentcode bot commented Jan 29, 2026

Uh oh!

augmentcode bot left a comment

Choose a reason for hiding this comment

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!

romange Jan 29, 2026

Choose a reason for hiding this comment

Uh oh!

abhijat Jan 29, 2026

Choose a reason for hiding this comment

Uh oh!

romange Jan 29, 2026

Choose a reason for hiding this comment

Uh oh!

abhijat Jan 29, 2026

Choose a reason for hiding this comment

Uh oh!

romange Jan 29, 2026

Choose a reason for hiding this comment

Uh oh!

abhijat Jan 30, 2026

Choose a reason for hiding this comment

Uh oh!

romange left a comment

Choose a reason for hiding this comment

Uh oh!

dranikpg left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

abhijat commented Jan 30, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

romange Jan 30, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants