Add pageSize and defaultPageSize

[jacob-carlborg]

These will give the size of a system page in bytes.

[dlang-bot]

Thanks for your pull request and interest in making D better, @jacob-carlborg! 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 fetch digger
dub run digger -- build "master + druntime#2952"

[Geod24]

Isn't it a bit weird to have those two values which are kind-of the same but not quite ?
Personally I find it confusing. pageSize seems like the "right" value, however it is never cached.

My intuition would be to just have pageSize and initialize from a crt_constructor and just use that everywhere. Usually I would go with lazy-initialization, but pageSize could be used everywhere in druntime (e.g. core.thread), so lazy initialization doesn't make sense.

[jacob-carlborg]

My intuition would be to just have pageSize and initialize from a crt_constructor and just use that everywhere. Usually I would go with lazy-initialization, but pageSize could be used everywhere in druntime (e.g. core.thread), so lazy initialization doesn't make sense.

I just didn't think of that 😊. I'll make the change.

[jacob-carlborg]

@Geod24 the point was actually to be able to use the page size in a static context, i.e. ubyte[defaultPageSize]. That is used in several places in std.experimental.allocator.

If an immutable variable doesn't have a value and I try to use it in a static context I get this error:

static variable pageSize cannot be read at compile time

Also, a function with pragma(crt_constructor) cannot initialize an immutable value. Looks like it's treated as a regular function by the compiler while shared static this has special treatment.

[Geod24]

the point was actually to be able to use the page size in a static context

But then isn't it a problematic assumption ? 4kb is the smallest page size on most arches / OSes (wiki link) but as seen in that link it can be configured to be higher, and as memory becomes cheaper it would make sense that this become more and more frequent.

Can you give me some link to those usages ?

Also, a function with pragma(crt_constructor) cannot initialize an immutable value. Looks like it's treated as a regular function by the compiler while shared static this has special treatment.

Which makes sense, because that function could be called again later, manually.
Andrei originally proposed extern(C) [shared] static this(). In hindsight that might have been better.

If an immutable variable doesn't have a value and I try to use it in a static context I get this error:

Yeah there's no way to make it work. immutable with initializer are hard-coded into the binary, which means that even if you could call that (extern(C)) function at compile time, you'd have the value of whatever system is building the runtime.

[jacob-carlborg]

But then isn't it a problematic assumption?

Yes, but that's how the code looks like. With this change it's not going to get any worse but it's a slight improvement.

There are currently 165 occurrences of the value 4096 in Phobos and 85 in druntime. I would guess that most of them refer to or are based on the page size. Going through them would be quite a big task and I don't think that should be included in porting D to iOS.

I can keep defaultPageSize in std.experimental.allocator, but as I mentioned there might be up to 250 places in the code that need to use this enum.

Can you give me some link to those usages ?

These three are the ones that caused some of the tests on iOS to fail. There are more uses of the value 4096 in std.experimental.allocator.

https://github.com/dlang/phobos/blob/7459693768ab63267b5b39eb7c1d4e1750c5abda/std/experimental/allocator/building_blocks/allocator_list.d#L826

https://github.com/dlang/phobos/blob/7459693768ab63267b5b39eb7c1d4e1750c5abda/std/experimental/allocator/building_blocks/allocator_list.d#L886

https://github.com/dlang/phobos/blob/7459693768ab63267b5b39eb7c1d4e1750c5abda/std/experimental/allocator/building_blocks/allocator_list.d#L953

[Geod24]

All of those links are in unittests though.

But if you look at the AscendingPageAllocator it actually does the right thing.

Going through them would be quite a big task and I don't think that should be included in porting D to iOS.

Yeah I don't think we should be putting that on you.

Looking at this link, it seems that 16k was not always the default. Wikipedia says A7 appeared in 2013 with iPhone5S, and software updates for this chip has been discontinued this year, so I suppose you have no intention to support it and 16k is a good minimum (BTW is the minimum supported version for your iOS port documented anywhere?).

So, revised suggestion:

• Rename the static variable to minimumPageSize and document that this is a minimum value available at CT but the actual value can only be obtained when the program starts;
• Apply the pageSize suggestion, set it in the shared static this. Since this is a leaf module it should just work (and nothing in druntime depends on it yet). Someone can later move it to crt_constructor if they wish to use it in code that runs before shared static this.

How does that sound ?

[jacob-carlborg]

But if you look at the AscendingPageAllocator it actually does the right thing.

Yes, but if you look further down you'll see that it uses the hard coded value 4096 for the alignment [1]. I don't know if the alignment have to match the page size or not.

BTW is the minimum supported version for your iOS port documented anywhere?

No. The answer is a bit complicated. When it comes to the hardware, the intention it to only support 64bit. That means that the oldest possible version would be iPhone 5S. I have tested on iPhone 6, which was the model after iPhone 5S. When it comes to the software, the oldest version to possible support is iOS 9 (this is when native TLS support was adde,d according to Dan Olson).

For any new code added, I have used the iOS 13.2 SDK as the base. When testing, I have compiled the code to target iOS 12.0. I have run the code on iPhone 6 running iOS 12.4.5.

When it comes to deciding the exact version we want to officially support I would say that highly depends on what the CI environment is going to provide.

Rename the static variable to minimumPageSize and document that this is a minimum value

Ok.

available at CT but the actual value can only be obtained when the program starts

Is that not clear enough know in the documentation?

Apply the pageSize suggestion, set it in the shared static this. Since this is a leaf module it should just work (and nothing in druntime depends on it yet). Someone can later move it to crt_constructor if they wish to use it in code that runs before shared static this.

Do we know that this value cannot change at runtime?

We could use crt_constructor and temporarily remove the immutability by casting.

[1] https://github.com/dlang/phobos/blob/130485849a0e13e995f7c8e8aa3135707896d8b5/std/experimental/allocator/building_blocks/ascending_page_allocator.d#L255

[jacob-carlborg]

Which makes sense, because that function could be called again later, manually.

A shared static constructor can be called again manually as well:

import std;

__gshared immutable int foo;
shared static this() @safe
{
    foo += 3;
    writeln("shared static this");
}

void main() @safe
{
    writefln!"foo=%s"(foo);
    _sharedStaticCtor_L4_C1();
    writefln!"foo=%s"(foo);
}

The above will print:

shared static this
foo=3
shared static this
foo=6

[Geod24]

🤦‍♂

[jacob-carlborg]

@Geod24 renamed to minimumPageSize. I chose to go with a function declared as crt_constructor and cast away immutable to initialize pageSize.

[Geod24]

The trick (having two functions with the same name) makes me a bit nervous, but it seems to work.

[Geod24]

A shared static constructor can be called again manually as well:
[...]

Good point, and thanks for the pointer, the fix is trivial: dlang/dmd#10834

[jacob-carlborg]

The trick (having two functions with the same name) makes me a bit nervous, but it seems to work.

It’s mostly to get the D mangling without hard coding it. It’s been possible forever, required for .di files. A couple of years ago Manu asked to get that supported in the same file as well and that was implemented.