On Monday, 25 January 2021 at 22:00:48 UTC, H. S. Teoh wrote:
> On Mon, Jan 25, 2021 at 09:26:28PM +0000, Tove via Digitalmars-d wrote:
>> On Monday, 25 January 2021 at 20:07:04 UTC, H. S. Teoh wrote:
>> > Relying on a GC-collected object to reclaim a resource (in this case a mmap mapping) is an anti-pattern that should be expunged from Phobos.
>> > 
>> > 	https://issues.dlang.org/show_bug.cgi?id=9501
> [...]
>> Fully agree... it was brought up ages
> At this point, a dub package or some other external module is probably a better bet.  I'd try my hand at a standalone 1-file replacement for std.mmfile,

Yeah, dub is totally okay.

On Monday, 25 January 2021 at 22:00:48 UTC, H. S. Teoh wrote:
> [...] I'm wary of submitting this to Phobos, though, because the current PR process is a gigantic timesink and time is something I do not have enough of.
>
> At this point, a dub package or some other external module is probably a better bet.  I'd try my hand at a standalone 1-file replacement for std.mmfile, but I've zero Windows dev experience so it will only be Posix-compatible.

If you do a PR and I remember looking for it, I'll test it on my Windows machine.

Corrected example:

import std.mmfile;

struct Database {
    string filename;
    MmFile file;

    this(string filename) {
        this.filename = filename;
        file = new MmFile(filename, MmFile.Mode.read, 0, null);
    }

    ~this() {
        if (file) {
            import core.memory : GC;
            destroy(file);
            GC.free(cast(void*)file);
        }
    }
}

//somewhere in code:
void main(string[] args) {
    Database db = Database(args[0]);
}

Note: you did not need to call GC.free. The destroy would have guaranteed unmapping internally. GC.free would only remove the class instance memory itself.

On Mon, Jan 25, 2021 at 10:44:41PM +0000, aberba via Digitalmars-d wrote:
> On Monday, 25 January 2021 at 22:00:48 UTC, H. S. Teoh wrote:
[...]
> > At this point, a dub package or some other external module is probably a better bet.  I'd try my hand at a standalone 1-file replacement for std.mmfile,
> 
> Yeah, dub is totally okay.

Dub is OK as a package manager, but I have serious difficulty with using it as a build system. It just goes against the grain of how I usually work, and has fundamental limitations with what I usually need to do, and so I have not invested much effort into it.

My current inclination is along the lines of Adam Ruppe's single-file, self-contained, dependency-free module that you can literally just copy into your workspace and use right away.  Over the years I've come to question the sacred cow of code reuse and the dependency hell it often entails (solving NP-complete problems, for example, should be something your program helps the user to do, not something that you have to do as a prerequisite to compiling/using the program!), and now prefer alternative approaches that involve simpler and more uniformly composable artifacts.

(Of course, if anybody wants to add dub support to my code, I'm more
than happy to accept patches.)


T

-- 
Marketing: the art of convincing people to pay for what they didn't need before which you fail to deliver after.

On Monday, 25 January 2021 at 22:00:48 UTC, H. S. Teoh wrote:
> On Mon, Jan 25, 2021 at 09:26:28PM +0000, Tove via Digitalmars-d wrote:
>> > [...]
> [...]
>> [...]
>
> I'd submit a PR for it, but based on the discussion in bugzilla, it probably won't go through, because (1) changing from a class to a struct will break pretty much *every* usage of it in existing code, and we're oh so afraid of breaking changes these days even if it's all for the better; (2) the anticipated back-n-forth on the ensuing PR discussion will probably wear me out and make me walk away as quickly as I approached.
>
> [...]

I think that sooner or later the problem of evolving Phobos should be tackled by the gatekeepers, providing at least a policy for incremental improvements.

I personally have a toe in that cold water, as a really simple PR, and it's stalled from ages waiting exactly for that.

On 1/25/21 9:55 PM, rikki cattermole wrote:

> Note: you did not need to call GC.free. The destroy would have guaranteed unmapping internally. GC.free would only remove the class instance memory itself.

This. GC is for memory cleanup, but not necessary to cleanup resources.

If you are calling GC.free in your code, likely you are doing something wrong.

-Steve

On Tuesday, 26 January 2021 at 15:16:01 UTC, Steven Schveighoffer wrote:
> On 1/25/21 9:55 PM, rikki cattermole wrote:
>
>> Note: you did not need to call GC.free. The destroy would have guaranteed unmapping internally. GC.free would only remove the class instance memory itself.
>
> This. GC is for memory cleanup, but not necessary to cleanup resources.
>
> If you are calling GC.free in your code, likely you are doing something wrong.
>
> -Steve

I took the example from an unittest, assuming that people writing phobos libs are way smarter than me :)


https://github.com/dlang/phobos/blob/fb7bfb2ba5a581cafb9e85ced8f8d67aab35f412/std/mmfile.d#L673

On Tuesday, 26 January 2021 at 17:06:41 UTC, Rumbu wrote:
> I took the example from an unittest, assuming that people writing phobos libs are way smarter than me :)
>
>
> https://github.com/dlang/phobos/blob/fb7bfb2ba5a581cafb9e85ced8f8d67aab35f412/std/mmfile.d#L673

Phobos code varies a lot in terms of quality and best-practice adherence. This unit test in particular was written 14 years ago [1], and has not meaningfully changed since then, except to replace the deprecated `delete` keyword with `destroy` + `GC.free`. [2] It is probably safe to assume that it would be different if it were written from scratch today.

[1] https://github.com/dlang/phobos/commit/6b069176ba798e3652a1ec5b04c0deea0f4f861f
[2] https://github.com/dlang/phobos/commit/9d8cb9fda2013d9cb69e67a3a726067f78875346

On 1/26/21 12:06 PM, Rumbu wrote:
> On Tuesday, 26 January 2021 at 15:16:01 UTC, Steven Schveighoffer wrote:
>> On 1/25/21 9:55 PM, rikki cattermole wrote:
>>
>>> Note: you did not need to call GC.free. The destroy would have guaranteed unmapping internally. GC.free would only remove the class instance memory itself.
>>
>> This. GC is for memory cleanup, but not necessary to cleanup resources.
>>
>> If you are calling GC.free in your code, likely you are doing something wrong.
>>
> 
> I took the example from an unittest, assuming that people writing phobos libs are way smarter than me :)
> 
> 
> https://github.com/dlang/phobos/blob/fb7bfb2ba5a581cafb9e85ced8f8d67aab35f412/std/mmfile.d#L673 

https://github.com/dlang/phobos/pull/7770

-Steve