June 25, 2015
1 day remaining
June 26, 2015
On Thursday, 25 June 2015 at 13:36:31 UTC, Dicebot wrote:
> 1 day remaining

FYI: considering http://forum.dlang.org/thread/mmhjqe$2mud$1@digitalmars.com there will be some delay between finishing the review period and starting voting process. It is ok to use that time for any additional comments :P
June 26, 2015
The Windows MMap allocator only keeps one HANDLE around, and creates a new one on each `allocate`. Thus, `deallocate` closes the latest handle, regardless of what it was actually passed, so it leaks.

If I'm reading the docs for `CreateFileMapping` right, you should be able to close the handle after calling `MapViewOfFile`; the internal data will persist until you unmap the memory region.
June 26, 2015
On 26-Jun-2015 17:51, Alex Parrill wrote:
> The Windows MMap allocator only keeps one HANDLE around, and creates a
> new one on each `allocate`. Thus, `deallocate` closes the latest handle,
> regardless of what it was actually passed, so it leaks.
>

Actually I don't see why Windows couldnt' just use VirtualAlloc w/o messing with files.

> If I'm reading the docs for `CreateFileMapping` right, you should be
> able to close the handle after calling `MapViewOfFile`; the internal
> data will persist until you unmap the memory region.

IIRC no you can't. I'd need to double check that though.

-- 
Dmitry Olshansky
June 26, 2015
On Friday, 26 June 2015 at 14:56:21 UTC, Dmitry Olshansky wrote:
> On 26-Jun-2015 17:51, Alex Parrill wrote:
>> The Windows MMap allocator only keeps one HANDLE around, and creates a
>> new one on each `allocate`. Thus, `deallocate` closes the latest handle,
>> regardless of what it was actually passed, so it leaks.
>>
>
> Actually I don't see why Windows couldnt' just use VirtualAlloc w/o messing with files.

Yea, VirtualAlloc seems like a better fit. (I don't actually know the windows API that well)

>> If I'm reading the docs for `CreateFileMapping` right, you should be
>> able to close the handle after calling `MapViewOfFile`; the internal
>> data will persist until you unmap the memory region.
>
> IIRC no you can't. I'd need to double check that though.

Here's the paragraph I'm reading:

Mapped views of a file mapping object maintain internal references to the object, and a file mapping object does not close until all references to it are released. Therefore, to fully close a file mapping object, an application must unmap all mapped views of the file mapping object by calling UnmapViewOfFile and close the file mapping object handle by calling CloseHandle. These functions can be called in any order.
June 29, 2015
On Friday, 26 June 2015 at 13:35:34 UTC, Dicebot wrote:
It is ok to use that time for any
> additional comments :P

I would second the suggestion for improvement in the documentation. The overview linked to in the original post is written assuming advanced programming knowledge. My understanding of allocation is pretty much limited to something could be on stack or heap (I had to Google what an allocator was). So I would say it could be improved by adding some explanation of the basics and why to use allocators instead of new or other techniques.

I had seen this when it was originally posted two weeks ago, but I came back around to read it again today. What brought me to look it over again was that I had come across the std.container.array package and I didn't understand how it was different from the built-in array (I still don't have a very good understanding there, but a little better). Anyway, as I was searching through the Learn forum, I came across a post from 2011 saying that std.container might change in the future based on Andrei's work on allocators. I take it this is the work it was referring to (especially after looking at the wikipedia page for allocators).

With that I think I only have two questions. Am I correct that the makeArray function is for allocating a dynamic array and not an std.container Array? Are there plans to refactor std.container with std.allocator in mind?
June 30, 2015
On Friday, 26 June 2015 at 15:23:25 UTC, Alex Parrill wrote:
> On Friday, 26 June 2015 at 14:56:21 UTC, Dmitry Olshansky wrote:
>> [...]
>
> Yea, VirtualAlloc seems like a better fit. (I don't actually know the windows API that well)
>
>> [...]
>
> Here's the paragraph I'm reading:
>
> Mapped views of a file mapping object maintain internal references to the object, and a file mapping object does not close until all references to it are released. Therefore, to fully close a file mapping object, an application must unmap all mapped views of the file mapping object by calling UnmapViewOfFile and close the file mapping object handle by calling CloseHandle. These functions can be called in any order.

https://github.com/andralex/phobos/pull/17
June 30, 2015
On Tuesday, 30 June 2015 at 06:07:14 UTC, Baz wrote:
> On Friday, 26 June 2015 at 15:23:25 UTC, Alex Parrill wrote:
>> On Friday, 26 June 2015 at 14:56:21 UTC, Dmitry Olshansky wrote:
>>> [...]
>>
>> Yea, VirtualAlloc seems like a better fit. (I don't actually know the windows API that well)
>>
>>> [...]
>>
>> Here's the paragraph I'm reading:
>>
>> Mapped views of a file mapping object maintain internal references to the object, and a file mapping object does not close until all references to it are released. Therefore, to fully close a file mapping object, an application must unmap all mapped views of the file mapping object by calling UnmapViewOfFile and close the file mapping object handle by calling CloseHandle. These functions can be called in any order.
>
> https://github.com/andralex/phobos/pull/17

By the way, the ddoc comment for mmap needs to be updated (not posix only).
I don't know what to write because initially i just wanted to build allocator under win and proposed to the mmap allocator for win without thinking more...

currently under win the only difference between a simple malloc is that the allocation happen on commitment...previously it was really an anonymous memory mapped file...).


July 10, 2015
On Friday, 12 June 2015 at 11:06:43 UTC, Dicebot wrote:
> The legendary allocator package by Andrei Alexandrescu has arrived at your doorsteps and kindly asks to let it into Phobos

Sorry for being late, I wanted to restate an idea that could be crucial for optimizations.
https://github.com/D-Programming-Language/druntime/pull/1183#issuecomment-77065554

This reminds me of the fact, that the IAllocator interface (std.allocator) should also have a strongly pure allocate function. This might allow the compiler to optimize allocations even with a dynamic dispatch allocator, because it assumes that an allocation doesn't have a side-effect and always returns "fresh" unaliased memory.

Chandler Carruth was talking about this problem at the last CppCon.
https://www.youtube.com/watch?v=fHNmRkzxHWs&t=3950
https://www.youtube.com/watch?v=fHNmRkzxHWs&t=4037

Now putting pure onto the allocate function of IAllocator would be a very harsh constraint for any implementation. So maybe we could employ a compiler hack, marking any IAllocator.allocate implementation as pure.

This optimization is really important, b/c it allows the compiler to ellide allocations when it can compute something using a temporary buffer.
It'd probably require a magic free as well, so that the compiler knows the lifetime and sees that data isn't escaped.
July 12, 2015
On 2015-06-12 13:06, Dicebot wrote:
> The legendary allocator package by Andrei Alexandrescu has arrived at
> your doorsteps and kindly asks to let it into Phobos
>
> http://wiki.dlang.org/Review/std.experimental.allocator
>
> Docs: http://erdani.com/d/phobos-prerelease/std_experimental_allocator.html
> Code:
> https://github.com/andralex/phobos/tree/allocator/std/experimental/allocator

I just looked at Andrei's dconf talk and started to look at the source code how this "hasMember" is used. To me it looks like in most cases the optional methods, like "deallocate", "owns", "expand" and so on, could instead be required methods. Allocators that don't support these methods would need to have dummy implementations. But at the same time it would be easier for the user of the allocators, doesn't need to use all these "static if".

The current way of using "hasMember" and "static if" seems to be a more complicated design.

-- 
/Jacob Carlborg