dmd icon indicating copy to clipboard operation
dmd copied to clipboard

Templated arrayshrinkfit

Open samrendra-singh opened this issue 8 months ago • 4 comments

This PR is for demonstrating the protoype implementation of the _d_arrayshrinkfit runtime hook. This implementation:

  • Creates a templated version of the hook: _d_arrayshrinkfitT in core/internal/array/capacity.d
  • Maintains compatibility by forwarding calls to the original implementation
  • Adds a basic unittest to verify the functionality
  • Preserves handling of D_TypeInfo version conditions

Implementation Details The implementation:

  1. Adds a new templated function _d_arrayshrinkfitT that forwards to the original implementation
  2. Updates object.d to use this templated version in the assumeSafeAppend function
  3. Maintains proper handling for D_TypeInfo version conditions
  4. Adds a unittest to verify the basic functionality

Testing I've added a simple unittest to verify the basic functionality. The test creates an array with extra capacity, applies the shrinkfit function, and checks that the array contents remain unchanged. While running the full druntime-test suite shows some failures in the profile subsystem, these appear unrelated to this change. I have added a screehshot capturing the error below. ScreenshotArrayShrinkFit

I have a few questions

  • Is the forwarding approach correct?
  • For full templated implementation, should I start by directly replacing the TypeInfo usage, or should I take a more gradual approach?

samrendra-singh avatar Apr 05 '25 15:04 samrendra-singh

Thanks for your pull request and interest in making D better, @solo-source! We are looking forward to reviewing it, and you should be hearing from a maintainer soon. Please verify that your PR follows this checklist:

  • My PR is fully covered with tests (you can see the coverage diff by visiting the details link of the codecov check)
  • My PR is as minimal as possible (smaller, focused PRs are easier to review than big ones)
  • I have provided a detailed rationale explaining my changes
  • New or modified functions have Ddoc comments (with Params: and Returns:)

Please see CONTRIBUTING.md for more information.


If you have addressed all reviews or aren't sure how to proceed, don't hesitate to ping us with a simple comment.

Bugzilla references

Your PR doesn't reference any Bugzilla issue.

If your PR contains non-trivial changes, please reference a Bugzilla issue or create a manual changelog.

Testing this PR locally

If you don't have a local development environment setup, you can use Digger to test this PR:

dub run digger -- build "master + dmd#21152"

dlang-bot avatar Apr 05 '25 15:04 dlang-bot

Hi @teodutu i tried to isolate and reproduce the failing behavior i was observing on my local machine at https://github.com/dlang/dmd/blob/b2926b6b2a884b8f283b3756122fe9513b1ac8ae/druntime/src/object.d#L4043 using a minimal reproduction listed below.

import std.stdio;

@system unittest
{
    int[] a = [1, 2, 3, 4];

    // Without assumeSafeAppend. Appending relocates.
    int[] b = a[0 .. 3];
    writeln("Case 1: Without assumeSafeAppend");
    writeln("  b.ptr before appending: ", b.ptr);
    b ~= 5;
    writeln("  a.ptr: ", a.ptr, "  b.ptr after appending: ", b.ptr);
    assert(a.ptr != b.ptr, "Expected b.ptr to differ from a.ptr after reallocation");

    // With assumeSafeAppend. Appending should not relocate.
    int[] c = a[0 .. 3];
    writeln("\nCase 2: With assumeSafeAppend");
    writeln("  c.ptr before assumeSafeAppend: ", c.ptr);
    c.assumeSafeAppend() ~= 5;
    writeln("  a.ptr: ", a.ptr, "  c.ptr after assumeSafeAppend: ", c.ptr);
    assert(a.ptr == c.ptr, "Expected c.ptr to remain equal to a.ptr");
}

void main() {} // Dummy main to allow standalone compilation

While executing this i am getting the following Output

C:\Source\D\dmd>C:\Source\D\dmd\generated\windows\release\64\dmd.exe "C:\Users\SamrendraSingh\Desktop\D Files\test_assumeSafeAppend.d" -unittest -debug -of=test_assumeSafeAppend.exe

C:\Source\D\dmd>"C:\Users\SamrendraSingh\Desktop\D Files\test_assumeSafeAppend.exe"
Case 1: Without assumeSafeAppend
  b.ptr before appending: 272DB0A0020
  a.ptr: 272DB0A0020  b.ptr after appending: 272DB0A1000

Case 2: With assumeSafeAppend
  c.ptr before assumeSafeAppend: 272DB0A0020
  a.ptr: 272DB0A0020  c.ptr after assumeSafeAppend: 272DB0A1030

core.exception.AssertError@C:\Users\SamrendraSingh\Desktop\D Files\test_assumeSafeAppend.d(21): Expected c.ptr to remain equal to a.ptr
----------------
0x00007FF6CC487737
0x00007FF6CC487090
0x00007FF6CC4713F2
0x00007FF6CC486E6D
0x00007FF6CC49A87E
0x00007FF6CC4997A7
0x00007FF6CC49EB84
0x00007FF6CC49C18C
0x00007FF6CC49EB0B
0x00007FF6CC499777
0x00007FF6CC49A720
0x00007FF6CC48D0A5
0x00007FF6CC48D01F
0x00007FF6CC48CF2A
0x00007FF6CC487299
0x00007FF6CC471424
0x00007FF6CC4C90D0
0x00007FFB81A6259D in BaseThreadInitThunk
0x00007FFB82EAAF38 in RtlUserThreadStart
1/1 modules FAILED unittests

C:\Source\D\dmd>

I am stumped, can’t pin point why this is happening, what are your thoughts on this?

samrendra-singh avatar Apr 07 '25 19:04 samrendra-singh

Try adding prints to the non-template _d_arrayshrinkfit and follow the execution on both master and your branch to figure out what's wrong. When running your code I saw the number of called dtors is also 0, so you can start from there. Find out why the array isn't properly finalised.

teodutu avatar Apr 12 '25 02:04 teodutu

i had accidently rebased this branch which resulted in unintended changes in the PR. I have taken the necessary steps to resolve it, and successfully reset the branch to the lastest commit related to this PR.

samrendra-singh avatar Apr 16 '25 09:04 samrendra-singh