[WIP] Add rcarray (using __RefCount) by lesderid · Pull Request #2646 · dlang/druntime

lesderid · 2019-06-18T05:06:52Z

This PR adds an array/slice type (rcarray!T) that behaves like built-in arrays (T[]) but manages its own memory through reference counting.

The implementation is largely based on #2348 but uses the upcoming __RefCount struct for reference counting. (This PR contains the __RefCount commits as they aren't merged yet, but please review them separately: #2608.)

Left to do:

Fix arrays of class types
Add more comments and documentation
Add factory function to work around no-arg ctors
Benchmark and compare vs similar types
Check for memory errors with asan

src/core/experimental/array.d

jacob-carlborg · 2019-06-18T08:32:40Z

What happens if you slice a rcarray and then append to the slice?

lesderid · 2019-06-18T15:56:07Z

What happens if you slice a rcarray and then append to the slice?

opOpAssign uses opBinary, which creates a copy of the array, so this behaves like built-in arrays.

PetarKirov · 2019-06-18T21:03:20Z

@lesderid thanks for picking up the torch of @edi33416's work!
Since this PR is based on #2348, can you include the last version of #2348 as the first commit in your branch, so that we can track the change that you made on top of it?

lesderid · 2019-06-18T21:34:30Z

Since this PR is based on #2348, can you include the last version of #2348 as the first commit in your branch, so that we can track the change that you made on top of it?

I'll do that but the diff probably won't be pretty...

src/core/experimental/array.d

lesderid · 2019-06-24T04:34:15Z

src/core/experimental/array.d

+ * Returns:
+ *      an empty `rcarray`
+ */
+auto make(Q : rcarray!T, T)(size_t initialCapacity = 0)


This factory function is at the module level. The reason for this is that this way one is able to specify the mutability qualifier, without repeating the name of the struct (i.e. auto ia = make!(immutable rcarray!int); instead of auto ia = rcarray!int.make!(immutable rcarray!int);). This does pollute the module and complicates selective imports.

The alternative is to simply not allow construction of const/immutable arrays through a factory function. That way, the factory function can be moved into the type (and optionally renamed, giving e.g auto a = rcarray!int.empty;). This might be fine, as I can't think of many legitimate use cases of constructing empty const/immutable arrays anyway.

(Sadly, a factory function is necessary to be able to create empty rcarrays without having to explicitly specify the initial capacity. We can't use the default constructor, as that would leave __RefCount, which allocates memory in its constructors, uninitialised).

Hmm wouldn't the name be confusing for people who know about stdx.allocator (https://dlang.org/phobos/std_experimental_allocator.html#make)?
What do other people think about the name?

We already have https://dlang.org/phobos/std_container_util.html#.make. I don't think it would be particularly confusing. That said, I still don't know if it's the best option.

src/core/experimental/array.d

wilzbach · 2019-06-28T09:28:11Z

src/core/experimental/array.d

+ * Returns:
+ *      an empty `rcarray`
+ */
+auto make(Q : rcarray!T, T)(size_t initialCapacity = 0)


Hmm wouldn't the name be confusing for people who know about stdx.allocator (https://dlang.org/phobos/std_experimental_allocator.html#make)?
What do other people think about the name?

src/core/experimental/array.d

Squashed commit of the following: commit ee96796 Author: Eduard Staniloiu <edi33416@gmail.com> Date: Fri Mar 29 15:16:46 2019 +0200 Fix invalid support bug commit 8e2a813 Author: Eduard Staniloiu <edi33416@gmail.com> Date: Thu Dec 6 18:21:57 2018 +0200 Make array usable in betterC commit ab1eef7 Author: Eduard Staniloiu <edi33416@gmail.com> Date: Tue Dec 4 16:29:00 2018 +0200 Leave bounds checking to underlying T[] support commit 82b1bbc Author: Eduard Staniloiu <edi33416@gmail.com> Date: Tue Dec 4 16:28:04 2018 +0200 Mark isShared as the msb inside the ref count commit 4410979 Author: Eduard Staniloiu <edi33416@gmail.com> Date: Thu Nov 29 14:14:54 2018 +0200 Use version (CoreUnittest) for stats allocator commit d0a8ae5 Author: Eduard Staniloiu <edi33416@gmail.com> Date: Tue Nov 27 13:52:22 2018 +0200 Refactor idup commit a144462 Author: Eduard Staniloiu <edi33416@gmail.com> Date: Tue Nov 27 13:50:26 2018 +0200 Refactor dup commit 97db995 Author: Eduard Staniloiu <edi33416@gmail.com> Date: Mon Nov 26 17:24:30 2018 +0200 Rename to `rcarray` commit 4ac6812 Author: Eduard Staniloiu <edi33416@gmail.com> Date: Mon Nov 26 17:15:46 2018 +0200 Comply with D_NoBoundsCheck commit 5d98a65 Author: Eduard Staniloiu <edi33416@gmail.com> Date: Mon Nov 26 16:26:41 2018 +0200 DStyle: Add space between version ( commit 68191fa Author: Eduard Staniloiu <edi33416@gmail.com> Date: Mon Nov 26 16:17:44 2018 +0200 Make `each` compatible with Phobos' `each` commit 65c0fde Author: Eduard Staniloiu <edi33416@gmail.com> Date: Tue Nov 20 21:05:08 2018 +0000 Change from labels to inline access specifiers commit 5fa4cbd Author: Eduard Staniloiu <edi33416@gmail.com> Date: Tue Nov 20 16:48:24 2018 +0200 Remove array's internal range API commit 025b50f Author: Eduard Staniloiu <edi33416@gmail.com> Date: Tue Nov 20 15:25:05 2018 +0200 Make global symbols private commit 86ea7ed Author: Eduard Staniloiu <edi33416@gmail.com> Date: Tue Nov 20 14:53:57 2018 +0200 Implement opEquals and remove global equals fn commit 60b3143 Author: Eduard Staniloiu <edi33416@gmail.com> Date: Tue Nov 20 14:27:06 2018 +0200 Allow toHash to infer safety commit 815942b Author: Eduard Staniloiu <edi33416@gmail.com> Date: Tue Nov 20 14:23:22 2018 +0200 Remove obsolete ctor commit 89b48a6 Author: Eduard Staniloiu <edi33416@gmail.com> Date: Mon Nov 12 22:01:55 2018 +0000 Remove range deps commit d9c02b0 Author: Eduard Staniloiu <edi33416@gmail.com> Date: Mon Nov 12 15:34:10 2018 +0000 Rename internal fields commit 6642656 Author: Eduard Staniloiu <edi33416@gmail.com> Date: Mon Nov 12 15:07:28 2018 +0000 Import Unqual from core traits commit 039483d Author: Eduard Staniloiu <edi33416@gmail.com> Date: Mon Nov 12 15:05:52 2018 +0000 Make length behave as builtin array's commit a206494 Author: Eduard Staniloiu <edi33416@gmail.com> Date: Sun Nov 11 21:46:11 2018 +0000 [WIP] Add @nogc array

This commit fixes rcarray for class types, storing class references (i.e. not class states). The previous code (both in dlang#2348 and in https://github.com/dlang-stdx/collections) was broken and allocated memory for a class *state* but used the memory to store *references*.

Found with ASan (LDC)

RazvanN7 · 2022-04-28T15:25:56Z

RC requires __mutable to be implemented. There is a reference to this PR here: https://issues.dlang.org/show_bug.cgi?id=23071

JinShil mentioned this pull request Jun 18, 2019

Add __RefCount struct #2608

Closed

lesderid force-pushed the rcarray branch from 1696e73 to e229024 Compare June 18, 2019 05:15

jacob-carlborg reviewed Jun 18, 2019

View reviewed changes

src/core/experimental/array.d Outdated Show resolved Hide resolved

jacob-carlborg reviewed Jun 18, 2019

View reviewed changes

src/core/experimental/array.d Outdated Show resolved Hide resolved

lesderid force-pushed the rcarray branch 3 times, most recently from 454c897 to 2797ba5 Compare June 18, 2019 23:35

n8sh reviewed Jun 24, 2019

View reviewed changes

src/core/experimental/array.d Outdated Show resolved Hide resolved

lesderid commented Jun 24, 2019

View reviewed changes

wilzbach reviewed Jul 5, 2019

View reviewed changes

lesderid force-pushed the rcarray branch from c3af32c to 1b22ab0 Compare July 5, 2019 15:35

edi33416 added 15 commits July 5, 2019 18:48

Add RefCount struct

4d300dc

Refactor

6f48e8f

Mark opAssign as return; return this

0a76317

Rename to _RefCount and place at topfile

691e432

Refactor test function

a075fd8

Remove superfluous const attr from ctor

f890aac

isUnique is false if uninitialized

5181f73

Fix style

bd81f83

Add ddoc

ad52cac

Rename to __RefCount

3cf2c50

Add support for shared qualifier

5ca5323

Add threading test

37cef07

Use factory function; remove std import

8075be3

Use shared type for refcount

b7b38c5

Remove dead code

6f218e8

lesderid added 12 commits July 5, 2019 18:48

[WIP] Add new rcarray

be782da

Fix use-after-free bug

08171c9

Found with ASan (LDC)

Improve comments, docs, and examples

923904f

Add factory function

ff48d1b

Fix style issues

500fd75

Move tests around

7151405

Clean up copy constructors

0c762f2

Use emplaceRef instead of emplace

a52dd34

Split opSlice example

c8d7c87

Add top-level overview examples

60360a5

lesderid force-pushed the rcarray branch from 1b22ab0 to 60360a5 Compare July 5, 2019 16:49

lesderid mentioned this pull request Jul 16, 2019

Add core.experimental.refcount (continuation of #2608) #2679

Closed

wilzbach mentioned this pull request Dec 18, 2019

Are we @nogc yet? dlang/project-ideas#56

Closed

dlang-bot added Needs Rebase needs a `git rebase` performed stalled labels May 27, 2021

RazvanN7 closed this Apr 28, 2022

Uh oh!

Comments

Conversation

lesderid commented Jun 18, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jacob-carlborg commented Jun 18, 2019

Uh oh!

lesderid commented Jun 18, 2019

Uh oh!

PetarKirov commented Jun 18, 2019

Uh oh!

lesderid commented Jun 18, 2019

Uh oh!

Uh oh!

lesderid Jun 24, 2019

Choose a reason for hiding this comment

Uh oh!

wilzbach Jun 28, 2019

Choose a reason for hiding this comment

Uh oh!

lesderid Jul 9, 2019

Choose a reason for hiding this comment

Uh oh!

Uh oh!

wilzbach Jun 28, 2019

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!

RazvanN7 commented Apr 28, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

lesderid commented Jun 18, 2019 •

edited

Loading