[Issue 18324] String switch lowering geneartes really long symbol names

2023-05-11 Thread d-bugmail--- via Digitalmars-d-bugs
https://issues.dlang.org/show_bug.cgi?id=18324

RazvanN  changed:

   What|Removed |Added

 CC||razvan.nitu1...@gmail.com

--- Comment #1 from RazvanN  ---
The longest mangled name for a switch that I've found has a 392 length. This
happens because the runtime __switch hook has variadic parameters which
represents the case strings. Maybe this could be improved by using a compile
time generated array of string cases.

--


[Issue 18324] String switch lowering geneartes really long symbol names

2022-12-17 Thread d-bugmail--- via Digitalmars-d-bugs
https://issues.dlang.org/show_bug.cgi?id=18324

Iain Buclaw  changed:

   What|Removed |Added

   Priority|P1  |P4

--


[Issue 18324] String switch lowering geneartes really long symbol names

2018-01-29 Thread d-bugmail--- via Digitalmars-d-bugs
https://issues.dlang.org/show_bug.cgi?id=18324

hst...@quickfur.ath.cx changed:

   What|Removed |Added

 CC||hst...@quickfur.ath.cx

--


Re: String Switch Lowering

2018-01-28 Thread Benjamin Thaut via Digitalmars-d

Am 27.01.2018 um 08:40 schrieb Walter Bright:


This clearly should be in bugzilla.



https://issues.dlang.org/show_bug.cgi?id=18324

--
Kind Regards
Benjamin Thaut


[Issue 18324] New: String switch lowering geneartes really long symbol names

2018-01-28 Thread d-bugmail--- via Digitalmars-d-bugs
https://issues.dlang.org/show_bug.cgi?id=18324

  Issue ID: 18324
   Summary: String switch lowering geneartes really long symbol
names
   Product: D
   Version: D2
  Hardware: x86
OS: Windows
Status: NEW
  Severity: enhancement
  Priority: P1
 Component: dmd
  Assignee: nob...@puremagic.com
  Reporter: c...@benjamin-thaut.de

The following symbol is generated from a string switch during the compilation
of phobos:

_D6object__T8__switchTyaVxAyaa7_43535436434454VxQwa7_45535435454454VxQBra7_4574632f474d54VxQCna7_4d5354374d4454VxQDja7_50535438504454VxQEfa9_417369612f4164656eVxQFfa9_417369612f42616b75VxQGfa9_417369612f44696c69VxQHfa9_417369612f486f7664VxQIfa9_417369612f4f6d736bVxQJfa9_417369612f4f72616cVxQKfa9_4574632f474d542b31VxQLfa9_4574632f474d542b32VxQMfa9_4574632f474d542b33VxQNfa9_4574632f474d542b34VxQOfa9_4574632f474d542b35VxQPfa9_4574632f474d542b36VxQQfa9_4574632f474d542b37VxQRfa9_4574632f474d542b38VxQSfa9_4574632f474d542b39VxQTfa9_4574632f474d542d31VxQUfa9_4574632f474d542d32VxQVfa9_4574632f474d542d33VxQWfa9_4574632f474d542d34VxQXfa9_4574632f474d542d35VxQYfa9_4574632f474d542d36VxQZfa9_4574632f474d542d37VxQBAfa9_4574632f474d542d38VxQBBga9_4574632f474d542d39VxQBCha10_417369612f416d6d616eVxQBDla10_417369612f4171746175VxQBEpa10_417369612f4368697461VxQBFta10_417369612f4468616b61VxQBGxa10_417369612f4475626169VxQBIba10_417369612f4b6162756cVxQBJfa10_417369612f4d61636175VxQBKja10_417369612f!
 
5161746172VxQBLna10_417369612f53656f756cVxQBMra10_417369612f546f6b796fVxQBNva10_417369612f546f6d736bVxQBOza10_4574632f474d542b3130VxQBQda10_4574632f474d542b3131VxQBRha10_4574632f474d542b3132VxQBSla10_4574632f474d542d3130VxQBTpa10_4574632f474d542d3131VxQBUta10_4574632f474d542d3132VxQBVxa10_4574632f474d542d3133VxQBXba10_4574632f474d542d3134VxQBYfa11_4166726963612f4a756261VxQBZla11_4166726963612f4c6f6d65VxQCAra11_417369612f416c6d617479VxQCBxa11_417369612f416e61647972VxQCDda11_417369612f4171746f6265VxQCEja11_417369612f426569727574VxQCFpa11_417369612f4272756e6569VxQCGva11_417369612f486562726f6eVxQCIba11_417369612f4b7577616974VxQCJha11_417369612f4d616e696c61VxQCKna11_417369612f4d7573636174VxQCLta11_417369612f526979616468VxQCMza11_417369612f536169676f6eVxQCOfa11_417369612f546169706569VxQCPla11_417369612f54656872616eVxQCQra11_417369612f5572756d7169VxQCRxa11_4575726f70652f4b696576VxQCTda11_4575726f70652f4f736c6fVxQCUja11_4575726f70652f52696761VxQCVpa11_4575726f70652f526f6d65VxQCWva1!
 
1_496e6469616e2f4d616865VxQCYba12_4166726963612f4163637261VxQCZja12_4166726963612f436169726fVxQDAra12_4166726963612f4365757461VxQDBza12_4166726963612f44616b6172VxQDDha12_4166726963612f4c61676f73VxQDEpa12_4166726963612f54756e6973VxQDFxa12_416d65726963612f4164616bVxQDHfa12_416d65726963612f4c696d61VxQDIna12_416d65726963612f4e6f6d65VxQDJva12_417369612f42616768646164VxQDLda12_417369612f4261687261696eVxQDMla12_417369612f42616e676b6f6bVxQDNta12_417369612f4261726e61756cVxQDPba12_417369612f426973686b656bVxQDQja12_417369612f436f6c6f6d626fVxQDRra12_417369612f49726b7574736bVxQDSza12_417369612f4a616b61727461VxQDUha12_417369612f4b617261636869VxQDVpa12_417369612f4b756368696e67VxQDWxa12_417369612f4d61676164616eVxQDYfa12_417369612f4e69636f736961VxQDZna12_417369612f52616e676f6f6eVxQEAva12_417369612f5462696c697369VxQECda12_417369612f5468696d706875VxQEDla12_417369612f59616b7574736bVxQEEta12_417369612f5965726576616eVxQEGba12_4575726f70652f4d616c7461VxQEHja12_4575726f70652f4d696e736bVxQEIra12_4575726f70652f5061726973VxQEJza12_4575726f70652f536f666961VxQELha12_45757!
 
26f70652f566164757aVxQEMpa12_496e6469616e2f436f636f73VxQENxa12_506163696669632f41706961VxQEPfa12_506163696669632f46696a69VxQEQna12_506163696669632f4775616dVxQERva12_506163696669632f4e697565VxQETda12_506163696669632f5472756bVxQEUla12_506163696669632f57616b65VxQEVta13_4166726963612f41736d657261VxQEXda13_4166726963612f42616d616b6fVxQEYna13_4166726963612f42616e677569VxQEZxa13_4166726963612f42616e6a756cVxQFBha13_4166726963612f426973736175VxQFCra13_4166726963612f446f75616c61VxQFEba13_4166726963612f486172617265VxQFFla13_4166726963612f4b6967616c69VxQFGva13_4166726963612f4c75616e6461VxQFIfa13_4166726963612f4c7573616b61VxQFJpa13_4166726963612f4d616c61626fVxQFKza13_4166726963612f4d617075746fVxQFMja13_4166726963612f4d6173657275VxQFNta13_4166726963612f4e69616d6579VxQFPda13_416d65726963612f4172756261VxQFQna13_416d65726963612f4261686961VxQFRxa13_416d65726963612f42656c656dVxQFTha13_416d65726963612f426f697365VxQFUra13_416d65726963612f4a756a7579VxQFWba13_416d65726963612f5369746b61VxQFXla13_4!
 
16d65726963612f5468756c65VxQFYva13_417369612f4173686761626174VxQGAfa13_417369612f43616c6375747461VxQGBpa13_417369612f44616d6173637573VxQGCza13_417369612f44757368616e6265VxQGEja13_417369612f4a61796170757261VxQGFta13_417369612f4b61746d616e6475VxQGHda13_4173

Re: String Switch Lowering

2018-01-28 Thread Seb via Digitalmars-d

On Saturday, 27 January 2018 at 23:12:01 UTC, H. S. Teoh wrote:
On Sat, Jan 27, 2018 at 09:22:07PM +, timotheecour via 
Digitalmars-d wrote: [...]

```
28  dscanner0x00010d59f428 
@safe void

...


I proposed a compile-time introspected getopt() replacement 
before, only to get laughed at by Andrei.


Do you remember his motivation?
FYI: since Nov 2017 Druntime has exactly what you are proposing

https://github.com/dlang/druntime/blob/master/src/core/internal/parseoptions.d

However, it's not really sophisticated as it's intended only to 
be used for druntime, but then again, no one is stopping you from 
using it:



```
struct Config
{
int a;
string b;
void help() @nogc nothrow {}
}

void main()
{
import core.internal.parseoptions, std.stdio;

string args = "a=42 b=foo";

Config conf;
conf.parseOptions(args);
conf.writeln;
}
```


https://run.dlang.io/is/GVclu2


Re: String Switch Lowering

2018-01-28 Thread Kagamin via Digitalmars-d

On Saturday, 27 January 2018 at 23:12:01 UTC, H. S. Teoh wrote:
I proposed a compile-time introspected getopt() replacement 
before


https://github.com/jasonwhite/darg this?


Re: String Switch Lowering

2018-01-27 Thread H. S. Teoh via Digitalmars-d
On Sat, Jan 27, 2018 at 03:48:19PM -0800, Timothee Cour wrote:
[...]
> eg `ldc -hash-threshold` would be 1 option.
[...]
> with a small threshold:
> 
> mangled:
> _D8analysis3run__T9shouldRunℂ0abf2284dd3
> 
> demangled:
> pure nothrow @nogc @safe immutable(char)[] analysis.run.shouldRun.ℂ0abf2284dd3
> 
> The `ℂ` symbol indicating hashing was applied because symbol size
> exceed threshold.
> The demangled version also would have that. A separate file (dmd
> -mangle_map=file) could be produced in the rare case a user wants to
> see the full 17KB mangled and demangled symbols mapped by ℂ0abf2284dd3
[...]

This gives me an idea.  A lot of the recent complaints about symbol size
appears to be coming from templates with string template arguments, and
encoding strings inside a symbol tend to quickly make its length
explode.  What if we changed the mangling scheme such that if a string
template argument exceeds a certain length, or if the number of string
arguments (or their accumulated length) exceeds a certain size, we hash
the string arguments instead, and use the hash in the symbol instead of
encoding the raw strings themselves?

Regardless, *some* form of symbol compression is necessary in D,
especially now that druntime is also moving in the direction of more
templated code. Rainer's backref patch helped in one common use case
(chained range functions), but the string argument case remains a source
of major symbol bloat.

I was also talking with Stefan Koch on github that we need to take up a
project to improve the way dmd handles templates. There's much room for
improvement, and with D's focus on heavy-duty compile-time features,
this is a major area where we can reap large benefits for the effort
invested.


T

-- 
Freedom of speech: the whole world has no right *not* to hear my spouting off!


Re: String Switch Lowering

2018-01-27 Thread Timothee Cour via Digitalmars-d
* This has nothing to do with name mangling.

Yes and no, these things are coupled. We can improve the situation by
forcing the size of mangled and demangled symbols to be < threshold,
eg `ldc -hash-threshold` would be 1 option.

example:

current mangled:
_D8analysis3run__T9shouldRunVAyaa20_666c6f61745f6f70657261746f725f636865636bZQChFQCaKxSQDh6config20StaticAnalysisConfigZ__T9__lambda4TQEbZQpFNaNbNiNfQEqZQEu

current demangled:
pure nothrow @nogc @safe immutable(char)[]
analysis.run.shouldRun!("float_operator_check").shouldRun(immutable(char)[],
ref 
const(analysis.config.StaticAnalysisConfig)).__lambda4!(immutable(char)[]).__lambda4(immutable(char)[])

with a small threshold:

mangled:
_D8analysis3run__T9shouldRunℂ0abf2284dd3

demangled:
pure nothrow @nogc @safe immutable(char)[] analysis.run.shouldRun.ℂ0abf2284dd3

The `ℂ` symbol indicating hashing was applied because symbol size
exceed threshold.
The demangled version also would have that. A separate file (dmd
-mangle_map=file) could be produced in the rare case a user wants to
see the full 17KB mangled and demangled symbols mapped by ℂ0abf2284dd3


On Sat, Jan 27, 2018 at 3:12 PM, H. S. Teoh via Digitalmars-d
 wrote:
> On Sat, Jan 27, 2018 at 09:22:07PM +, timotheecour via Digitalmars-d 
> wrote:
> [...]
>> ```
>> 28  dscanner0x00010d59f428 @safe void
>> std.getopt.getoptImpl!(std.getopt.config, immutable(char)[], bool*,
>> immutable(char)[], bool*, immutable(char)[], bool*, immutable(char)[],
>> bool*, immutable(char)[], bool*, immutable(char)[], bool*,
>> immutable(char)[], bool*, immutable(char)[], bool*, immutable(char)[],
>> bool*, immutable(char)[], bool*, immutable(char)[], bool*,
>> immutable(char)[], bool*, immutable(char)[], bool*, immutable(char)[],
>> bool*, immutable(char)[], bool*, immutable(char)[], immutable(char)[]*,
>> immutable(char)[], immutable(char)[]*, immutable(char)[], bool*,
>> immutable(char)[], immutable(char)[][]*, immutable(char)[], bool*,
>> immutable(char)[], bool*, immutable(char)[], bool*, immutable(char)[],
>> bool*).getoptImpl(ref immutable(char)[][], ref std.getopt.configuration, ref
>> std.getopt.GetoptResult, ref std.getopt.GetOptException,
>> void[][immutable(char)[]], void[][immutable(char)[]], std.getopt.config,
>> immutable(char)[], bool*, immutable(char)[], bool*, immutable(char)[],
>> bool*, immutable(char)[], bool*, immutable(char)[], bool*,
>> immutable(char)[], bool*, immutable(char)[], bool*, immutable(char)[],
>> bool*, immutable(char)[], bool*, immutable(char)[], bool*,
>> immutable(char)[], bool*, immutable(char)[], bool*, immutable(char)[],
>> bool*, immutable(char)[], bool*, immutable(char)[], bool*,
>> immutable(char)[], immutable(char)[]*, immutable(char)[],
>> immutable(char)[]*, immutable(char)[], bool*, immutable(char)[],
>> immutable(char)[][]*, immutable(char)[], bool*, immutable(char)[], bool*,
>> immutable(char)[], bool*, immutable(char)[], bool*) + 460
>> ```
>>
>> https://dlang.org/blog/2017/12/20/ds-newfangled-name-mangling/ doesn't
>> seem to help in cases like that
>
> This has nothing to do with name mangling. The mangling itself may be
> relatively small (and probably is, judging from the amount of repetition
> in the signature above), but what you're looking at is the *demangled*
> identifier. That's going to be big no matter what, unless we
> fundamentally change the way getopt() is implemented.
>
> I proposed a compile-time introspected getopt() replacement before, only
> to get laughed at by Andrei.  So I guess that means, don't expect to see
> that in Phobos anytime soon.  But I might post the code on github
> sometime for those who would benefit from it.  Basically, instead of
> taking a ridiculously long argument list, you create a struct whose
> members (together with some UDAs) define what the options are, any
> associated help text, etc., and just call it with the struct type as
> argument.  It does its thing, and returns the struct populated with the
> values retrieved from the command-line.  There are a few more features,
> but that's the gist of it.
>
>
> T
>
> --
> There is no gravity. The earth sucks.



Re: String Switch Lowering

2018-01-27 Thread H. S. Teoh via Digitalmars-d
On Sat, Jan 27, 2018 at 09:22:07PM +, timotheecour via Digitalmars-d wrote:
[...]
> ```
> 28  dscanner0x00010d59f428 @safe void
> std.getopt.getoptImpl!(std.getopt.config, immutable(char)[], bool*,
> immutable(char)[], bool*, immutable(char)[], bool*, immutable(char)[],
> bool*, immutable(char)[], bool*, immutable(char)[], bool*,
> immutable(char)[], bool*, immutable(char)[], bool*, immutable(char)[],
> bool*, immutable(char)[], bool*, immutable(char)[], bool*,
> immutable(char)[], bool*, immutable(char)[], bool*, immutable(char)[],
> bool*, immutable(char)[], bool*, immutable(char)[], immutable(char)[]*,
> immutable(char)[], immutable(char)[]*, immutable(char)[], bool*,
> immutable(char)[], immutable(char)[][]*, immutable(char)[], bool*,
> immutable(char)[], bool*, immutable(char)[], bool*, immutable(char)[],
> bool*).getoptImpl(ref immutable(char)[][], ref std.getopt.configuration, ref
> std.getopt.GetoptResult, ref std.getopt.GetOptException,
> void[][immutable(char)[]], void[][immutable(char)[]], std.getopt.config,
> immutable(char)[], bool*, immutable(char)[], bool*, immutable(char)[],
> bool*, immutable(char)[], bool*, immutable(char)[], bool*,
> immutable(char)[], bool*, immutable(char)[], bool*, immutable(char)[],
> bool*, immutable(char)[], bool*, immutable(char)[], bool*,
> immutable(char)[], bool*, immutable(char)[], bool*, immutable(char)[],
> bool*, immutable(char)[], bool*, immutable(char)[], bool*,
> immutable(char)[], immutable(char)[]*, immutable(char)[],
> immutable(char)[]*, immutable(char)[], bool*, immutable(char)[],
> immutable(char)[][]*, immutable(char)[], bool*, immutable(char)[], bool*,
> immutable(char)[], bool*, immutable(char)[], bool*) + 460
> ```
> 
> https://dlang.org/blog/2017/12/20/ds-newfangled-name-mangling/ doesn't
> seem to help in cases like that

This has nothing to do with name mangling. The mangling itself may be
relatively small (and probably is, judging from the amount of repetition
in the signature above), but what you're looking at is the *demangled*
identifier. That's going to be big no matter what, unless we
fundamentally change the way getopt() is implemented.

I proposed a compile-time introspected getopt() replacement before, only
to get laughed at by Andrei.  So I guess that means, don't expect to see
that in Phobos anytime soon.  But I might post the code on github
sometime for those who would benefit from it.  Basically, instead of
taking a ridiculously long argument list, you create a struct whose
members (together with some UDAs) define what the options are, any
associated help text, etc., and just call it with the struct type as
argument.  It does its thing, and returns the struct populated with the
values retrieved from the command-line.  There are a few more features,
but that's the gist of it.


T

-- 
There is no gravity. The earth sucks.


Re: String Switch Lowering

2018-01-27 Thread Timothee Cour via Digitalmars-d
but this should be handled at the compiler level, with no change in
standard library getopt, eg using a hashing scheme (cf `ldc
-hashthres`)

On Sat, Jan 27, 2018 at 2:38 PM, Kagamin via Digitalmars-d
 wrote:
> IIRC several years ago somebody created a dub package with DbI getopt. I
> think it wouldn't suffer from this issue.


Re: String Switch Lowering

2018-01-27 Thread Kagamin via Digitalmars-d
IIRC several years ago somebody created a dub package with DbI 
getopt. I think it wouldn't suffer from this issue.


Re: String Switch Lowering

2018-01-27 Thread timotheecour via Digitalmars-d

On Saturday, 27 January 2018 at 10:38:46 UTC, Kagamin wrote:

dmd


see also this horrendous stacktrace when calling getopt with a 
bad argument:


full stacktrace:

https://gist.github.com/timotheecour/d6b623bd3d223f5d958cd86adffd7807

just 1 line of this stacktrace:

```
28  dscanner0x00010d59f428 @safe 
void std.getopt.getoptImpl!(std.getopt.config, immutable(char)[], 
bool*, immutable(char)[], bool*, immutable(char)[], bool*, 
immutable(char)[], bool*, immutable(char)[], bool*, 
immutable(char)[], bool*, immutable(char)[], bool*, 
immutable(char)[], bool*, immutable(char)[], bool*, 
immutable(char)[], bool*, immutable(char)[], bool*, 
immutable(char)[], bool*, immutable(char)[], bool*, 
immutable(char)[], bool*, immutable(char)[], bool*, 
immutable(char)[], immutable(char)[]*, immutable(char)[], 
immutable(char)[]*, immutable(char)[], bool*, immutable(char)[], 
immutable(char)[][]*, immutable(char)[], bool*, 
immutable(char)[], bool*, immutable(char)[], bool*, 
immutable(char)[], bool*).getoptImpl(ref immutable(char)[][], ref 
std.getopt.configuration, ref std.getopt.GetoptResult, ref 
std.getopt.GetOptException, void[][immutable(char)[]], 
void[][immutable(char)[]], std.getopt.config, immutable(char)[], 
bool*, immutable(char)[], bool*, immutable(char)[], bool*, 
immutable(char)[], bool*, immutable(char)[], bool*, 
immutable(char)[], bool*, immutable(char)[], bool*, 
immutable(char)[], bool*, immutable(char)[], bool*, 
immutable(char)[], bool*, immutable(char)[], bool*, 
immutable(char)[], bool*, immutable(char)[], bool*, 
immutable(char)[], bool*, immutable(char)[], bool*, 
immutable(char)[], immutable(char)[]*, immutable(char)[], 
immutable(char)[]*, immutable(char)[], bool*, immutable(char)[], 
immutable(char)[][]*, immutable(char)[], bool*, 
immutable(char)[], bool*, immutable(char)[], bool*, 
immutable(char)[], bool*) + 460

```

https://dlang.org/blog/2017/12/20/ds-newfangled-name-mangling/ 
doesn't seem to help in cases like that




Re: String Switch Lowering

2018-01-27 Thread Kagamin via Digitalmars-d

dmd


Re: String Switch Lowering

2018-01-27 Thread Benjamin Thaut via Digitalmars-d

On Saturday, 27 January 2018 at 07:40:16 UTC, Walter Bright wrote:


This clearly should be in bugzilla.


As phobos or dmd bug?


Re: String Switch Lowering

2018-01-26 Thread Walter Bright via Digitalmars-d

On 1/25/2018 10:21 AM, Benjamin Thaut wrote:
So I was thinking to myself: Is it really a good idea to lower string switches 
to a template if it results in such symbols? This symbol alone takes 17815 Bytes.


If we think this is a good idea, should we rewrite this particular string switch 
to use a associative array instead to avoid the overly long symbol name?


This clearly should be in bugzilla.



Re: String Switch Lowering

2018-01-25 Thread Steven Schveighoffer via Digitalmars-d

On 1/25/18 1:41 PM, H. S. Teoh wrote:

On Thu, Jan 25, 2018 at 07:21:29PM +0100, Benjamin Thaut via Digitalmars-d 
wrote:



If we think this is a good idea, should we rewrite this particular
string switch to use a associative array instead to avoid the overly
long symbol name?

[...]

I believe the original idea behind using a template for string switches
was to allow the possibility for the implementation to be smarter about
how to implement the switch (IIRC, string switches used to be
implemented as a runtime function). Supposedly object.__switch could
analyze the list of strings at compile-time and generate a perfect hash
or something, to maximize runtime performance.


I believe that when the number of cases is small enough, the binary 
search of the strings is done recursively to allow full optimization. 
And these symbols should be relatively small.


But I think if the number of strings is large enough, it calls the same 
runtime function as before (essentially). But here we have a problem -- 
the wrapper for this function is essentially this giant symbol that 
generates the string array locally, and then calls the real function. 
It's a huge cost just for a simple inline-able wrapper to another call.


See the code here:
https://github.com/dlang/druntime/blob/master/src/object.d#L3873

The function that does the real work is __switchSearch, which is only 
templated on the char type. It's going to be very very infrequent that 
the exact same switch case list is used in multiple places, meaning you 
are paying a huge cost for essentially memoizing these string lists.


I think we need some way to mark a function as following a different 
mangling, or maybe even just avoid the whole memoization, do everything 
inline.


-Steve


Re: String Switch Lowering

2018-01-25 Thread H. S. Teoh via Digitalmars-d
On Thu, Jan 25, 2018 at 11:42:03AM -0700, Jonathan M Davis via Digitalmars-d 
wrote:
> On Thursday, January 25, 2018 19:21:29 Benjamin Thaut via Digitalmars-d 
> wrote:
> > If we think this is a good idea, should we rewrite this particular
> > string switch to use a associative array instead to avoid the overly
> > long symbol name?
> 
> That particular switch statement is in a function that's deprecated
> and scheduled to be completely removed in about six months, so I don't
> see much point in worrying about it unless it's causing serious
> problems, and while that symbol name is stupidly long, AFAIK, it's not
> really causing any problems.

I haven't verified this yet, but I suspect that this may be (one of?)
the cause(s) of my recent woes with dmd's memory usage on low-memory
systems. If indeed this is the cause, then yes, it *is* causing problems
for me and anyone else who wants to use dmd on a low-memory system (this
sounds almost like an oxymoron these days!). And since it's deprecated,
perhaps it wouldn't hurt to hack it to use an AA instead so that in the
meantime Phobos doesn't consume inordinate amounts of memory just to
compile.


> I never would have thought that a switch statement would even have a
> symbol associated with it though. Clearly, I have no clue about how
> they're implemented.
[...]

IIRC they used to be a runtime function in druntime, but recently got
changed into a template, ostensibly for more flexibility in how they're
implemented. Apparently the original function was pretty lousy in terms
of performance. The new object.__switch is supposedly better, but the
expense of ridiculously-long symbols in the binary. Some days you just
can't win. :-/


T

-- 
Two wrongs don't make a right; but three rights do make a left...


Re: String Switch Lowering

2018-01-25 Thread H. S. Teoh via Digitalmars-d
On Thu, Jan 25, 2018 at 07:21:29PM +0100, Benjamin Thaut via Digitalmars-d 
wrote:
> _D6object__T8__switchTyaVxAyaa7_[...snip ridiculously long symbol...]
> 
> The first time I encountered this symbol in phobos I though: WTF? Then
> I tried to demangle it:
> core.exception.RangeError@src\core\demangle.d(230): Range violation

LOL! This reminds me of the days before Rainer's symbol backreferencing
PR was merged, where a UFCS chain of range algorithms causes exponential
growth in symbol length. This one isn't exponential, but it *is* still
ridiculously long.  We need to fix it. :-D


> I was then quickly informed by Rainer Scheutze what the correct
> demangling for this symbols is:
> 
> pure nothrow @nogc @safe int object.__switch!(immutable(char), "CST6CDT",
> "EST5EDT", "Etc/GMT", "MST7MDT", "PST8PDT", "Asia/Aden", "Asia/Baku",
[... snip ridiculously long template argument list ...]
> "America/Argentina/Rio_Gallegos",
> "America/North_Dakota/New_Salem").__switch(scope const(immutable(char)[]))
> 
> So I was thinking to myself: Is it really a good idea to lower string
> switches to a template if it results in such symbols? This symbol
> alone takes 17815 Bytes.

I think this is part of a much larger issue that we need to tackle, that
is, long template argument lists (esp. since D allows you to directly
manipulate these lists aka tuples aka AliasSeq, so user code is liable
to generate large numbers of these things with potentially very long
lengths).

I don't have a clear solution yet, but my initial thought is that in
cases like these, where the list is basically unique, all that's
*really* required of the generated symbol is that it be unique. There is
really no need to go encoding every last detail into the symbol name, as
if the first 1000 bytes or so of the symbol isn't probably already
enough to disambiguate it from every other symbol in the program.  If we
could somehow detect or annotate these cases as merely requiring a
unique symbol, then we could just substitute the whole monstrous thing
with a hash, like an MD5 or SHA checksum, which will be much less than
100 bytes.


> If we think this is a good idea, should we rewrite this particular
> string switch to use a associative array instead to avoid the overly
> long symbol name?
[...]

I believe the original idea behind using a template for string switches
was to allow the possibility for the implementation to be smarter about
how to implement the switch (IIRC, string switches used to be
implemented as a runtime function). Supposedly object.__switch could
analyze the list of strings at compile-time and generate a perfect hash
or something, to maximize runtime performance.

IMO the real fix ought to be to make the compiler somehow recognize
these cases and generate shorter symbols for them, rather than
hard-coding the Phobos code to use AAs, though admittedly, the latter
may probably a necessary stop-gap measure in the meantime.

(On which note, I wonder if you may have inadvertently found the source
of my recent dmd memory usage woes... a symbol like this in a commonly
imported module in Phobos like std.datetime would explain why recently I
suddenly can't compile Phobos anymore on a low-memory system without
invoking the kernel OOM killer, or why even the most trivial of projects
take ridiculous amounts of memory to compile.)


T

-- 
The volume of a pizza of thickness a and radius z can be described by the 
following formula: pi zz a. -- Wouter Verhelst



Re: String Switch Lowering

2018-01-25 Thread Benjamin Thaut via Digitalmars-d

Am 25.01.2018 um 19:42 schrieb Jonathan M Davis:


That particular switch statement is in a function that's deprecated and
scheduled to be completely removed in about six months, so I don't see much
point in worrying about it unless it's causing serious problems, and while
that symbol name is stupidly long, AFAIK, it's not really causing any
problems.



The main problem is binary size for shared library verisons of phobos. 
The overly long symbols names contribute significantly to binary size as 
for both exports and imports. The full symbol name is stored in the dll 
and the exe that uses the dll.


--
Kind Regards
Benjamin Thaut


Re: String Switch Lowering

2018-01-25 Thread Jonathan M Davis via Digitalmars-d
On Thursday, January 25, 2018 19:21:29 Benjamin Thaut via Digitalmars-d 
wrote:
> If we think this is a good idea, should we rewrite this particular
> string switch to use a associative array instead to avoid the overly
> long symbol name?

That particular switch statement is in a function that's deprecated and
scheduled to be completely removed in about six months, so I don't see much
point in worrying about it unless it's causing serious problems, and while
that symbol name is stupidly long, AFAIK, it's not really causing any
problems.

I never would have thought that a switch statement would even have a symbol
associated with it though. Clearly, I have no clue about how they're
implemented.

- Jonathan M Davis



String Switch Lowering

2018-01-25 Thread Benjamin Thaut via Digitalmars-d