Skip to content

Draft: reduce size of IntegerExp for small numbers... - #23662

Draft
rainers wants to merge 5 commits into
dlang:masterfrom
rainers:integerexp_16_64
Draft

rainers wants to merge 5 commits into
dlang:masterfrom
rainers:integerexp_16_64

Conversation

@rainers

@rainers rainers commented Aug 20, 2026

Copy link
Copy Markdown
Member

.. using the padding space inside Expression

  • make IntegerExp abstract, implemented by Integer64Exp and Integer16Exp with appropriate storage space for given value
  • use IntegerExp.create instead of new IntegerExp in parsing and semantic
  • use emplace!IntegerExp64 in interpreter, it uses a temporary space in a region allocator anyway

The first commit removes some strange modifications of the value that no test triggers. The only remaining setInteger calls are assignInPlace in CTFE and when restricting the value of a bitfield.

@rikkimax @ibuclaw This might be extended for cent-literals discussed in #23575

…Exp with appropriate storage space for given value

- use IntegerExp.create instead of new IntegerExp in parsing and semantic
- user emplace!IntegerExp64 in interpreter
@github-actions

github-actions Bot commented Aug 20, 2026

Copy link
Copy Markdown

DMD perf check

Metric Base PR Δ
compile hello.d (instr) 214.1 M 213.3 M -0.364%
compile hello.d -O -release (instr) 232.4 M 231.6 M -0.339%
compile Phobos (instr) 5,087.8 M 5,011.7 M -1.496%
compile Phobos codegen (instr) 1,469.6 M 1,463.5 M -0.414%
compile vibe.d (instr) 15,012.2 M 14,795.9 M -1.441%
dmd binary size (stripped) 6.95 MB 7.01 MB +0.74%
Breakdown — compile hello.d
Phase (wall, self time) Base PR Δ
parse 36.1 ms 32.8 ms -9.10%
sema_other 13.6 ms 13.8 ms +1.54%
sema1 10.1 ms 10.3 ms +1.71%
sema3 5.8 ms 5.7 ms -1.36%
codegen 2.0 ms 1.9 ms -0.92%
Breakdown — compile Phobos

-76.1 M instructions: frontend -70.0 M (-1.94%), codegen -6.1 M (-0.41%)

Phase (wall, self time) Base PR Δ
sema1 214 ms 218 ms +2.01%
sema_other 195 ms 196 ms +0.96%
codegen 392 ms 393 ms +0.29%
sema3 562 ms 561 ms -0.14%
parse 114 ms 113 ms -0.52%
sema2 1.9 ms 1.5 ms -25.28%
ctfe 13.9 ms 14.3 ms +2.58%
inline 4.2 ms 4.3 ms +1.22%
All measurements
Metric Base PR Δ
compile hello.d (instr) 214.1 M 213.3 M -0.364%
compile hello.d -O -release (instr) 232.4 M 231.6 M -0.339%
compile Phobos (instr) 5,087.8 M 5,011.7 M -1.496%
compile Phobos codegen (instr) 1,469.6 M 1,463.5 M -0.414%
compile vibe.d (instr) 15,012.2 M 14,795.9 M -1.441%
dmd binary size (stripped) 6.95 MB 7.01 MB +0.74%
hello binary size (stripped) 0.72 MB 0.72 MB 0.00%
peak RSS (compile hello.d) 43.40 MB 43.42 MB +0.05%
peak RSS (compile Phobos) 619.3 MB 617.2 MB -0.35%
peak RSS (compile vibe.d) 1913 MB 1914 MB +0.03%
compile dmd itself (wall) 12.0 s 12.0 s -0.05%
compile hello.d (wall) 67.6 ms 64.6 ms -4.44%
compile Phobos (wall) 1,496 ms 1,502 ms +0.39%

45a1e24 vs merge-base 363ee8b · about these metrics

@rikkimax

Copy link
Copy Markdown
Contributor

I'm fine with doing something like this, my concern for cent/ucent is politically it doesn't look like it'll be accepted at this point. It was never considered that we could just throw an LLM at it.

@rainers

rainers commented Aug 22, 2026

Copy link
Copy Markdown
Member Author

Even though this improves the speed a bit (I suspect memcpy with non-constant size is more expensive than a virtual call that can make the copy of 32-40 bytes with just a few instructions), I'm wondering why there is no measured drop in memory here. For my usual test building all phobos unittests, I get:

  • about 21 million IntegerExp ctors run
  • about 20 million emplaced during CTFE, so on the stack or in CTFE region
  • about 9.5 million copies made via copy()
  • that leaves about 10.5 million instances on the heap
  • more than 99% of the values are in the short range

If 8 bytes per IntegerExp is saved that results in about 80 MB reduced memory (of 11 GB), and that is roughly what I'm seeing locally.
Maybe phobos unittests are exaggerating the use of integers, building phobos alone creates only about 300k instances on the heap.

@ibuclaw

ibuclaw commented Aug 22, 2026

Copy link
Copy Markdown
Member

ACK.

I still think IntegerExp shouldn't be broken up, rather just make it able to handle N-byte integers.

extern (C++) final class IntegerExp : Expression
{
    // Number of dinteger_ts in value
    // assert(length_ > 0)
    alias length_ = astNodeBitFields;

    private union {
        dinteger_t value_;
        dinteger_t* extvalue_;
    }

    dinteger_t value() const {
        return length < 2 ? value_
            : extvalue_[0];
    }
    Cent value() const {
        return length < 2 ? Cent(lo:value_)
            : *cast(Cent*)extvalue_;
    }
    BigInt value() const {
        return length < 2 ? BigInt(value_)
            : BigInt(extvalue_, length_);
    }
    //...
}

@rainers

rainers commented Aug 24, 2026

Copy link
Copy Markdown
Member Author

I still think IntegerExp shouldn't be broken up, rather just make it able to handle N-byte integers.

The derived classes were meant to reduce the necessary memory, but that did not turn out as much as I hoped for. So the actual implementation is probably not so important.

Having to deal with multiple return types (BTW: need to have different method names) might be troublesome, too, as it might silently truncate values.

I'm not convinced that the compiler needs to know about cent integers, or even arbitrarily-sized integers, though. I would prefer being able to write them as a library type that seamlessly integrates with internal types including implicit conversions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants