[Arm64/Unix] WIP-DoNotMerge-Draft 64K static support #10891

sdmaclea · 2017-04-11T15:44:33Z

No description provided.

sdmaclea · 2017-04-11T15:49:09Z

src/gc/gcsvr.cpp

@@ -4,6 +4,7 @@



+#define OS_PAGE_SIZE 0x1000
 #include "common.h"


This creates two definitions for OS_PAGE_SIZE in the same project. Needs to be actual current OS_PAGE_SIZE.

sdmaclea · 2017-04-11T15:49:26Z

src/gc/gcwks.cpp

@@ -4,6 +4,7 @@



+#define OS_PAGE_SIZE 0x1000


Same as above

sdmaclea · 2017-04-11T15:50:24Z

src/inc/switches.h

@@ -76,7 +76,7 @@
    #define USE_UPPER_ADDRESS       0

 #elif defined(_TARGET_AMD64_) || defined(_TARGET_ARM64_)
-    #define PAGE_SIZE               0x1000
+    #define PAGE_SIZE               0x10000


Page size will become dynamic

sdmaclea · 2017-04-11T15:51:12Z

src/pal/src/include/pal/virtual.h

@@ -58,7 +58,7 @@ enum VIRTUAL_CONSTANTS
    VIRTUAL_EXECUTE,
    VIRTUAL_EXECUTE_READ,

-    VIRTUAL_PAGE_SIZE       = 0x1000,
+    VIRTUAL_PAGE_SIZE       = 0x10000,


Virtual page size will become dynamic an so will VIRTUAL_PAGE_SIZE_MASK

sdmaclea · 2017-04-11T15:51:52Z

src/utilcode/util.cpp

@@ -551,6 +551,8 @@ LPVOID ClrVirtualAllocAligned(LPVOID lpAddress, SIZE_T dwSize, DWORD flAllocatio

 #else // !FEATURE_PAL

+    if(alignment < PAGE_SIZE) alignment = PAGE_SIZE;
+
    // UNIXTODO: Add a specialized function to PAL so that we don't have to waste memory


64K page size will increase the amount of wasted memory

sdmaclea · 2017-04-11T15:55:18Z

src/zap/zapimage.cpp

@@ -1547,7 +1547,7 @@ void ZapImage::OutputTables()

 #if defined(FEATURE_PAL)
    // PAL library requires native image sections to align to page bounaries.
-    SetFileAlignment(0x1000);
+    SetFileAlignment(0x10000);


Section alignment to match maximum supported page size.

I believe that this will make native images a lot bigger. I am not sure whether this would be acceptable.

How are the native binary formats like ELF dealing with the 64k pages? Do they have the sections aligned at 64k boundaries as well?

@jkotas Based on what I read in golang/go#10180 (cited by @vielmetti). I think ELF will be aligning the sections to 64K page boundaries. However the ELF file size will not be padded to fill 64K, it will truncated as needed.

It is possible we could support file with 4K alignment, but PAL loader would need to be updated. This seems feasible since we have RELOC_PAGE_SIZE set to 4K.

@sdmaclea what do you mean by RELOC_PAGE_SIZE? I don't see such a symbol defined anywhere.

@janvorli src/zap/zaprelocs.h:#define RELOCATION_PAGE_SIZE 0x1000

@jkotas For the smallest files the size is growing about the same amount

12288 4KAligned/System.Xml.XPath.ni.dll 196608 64KAligned/System.Xml.XPath.ni.dll

@jkotas I'll see if I can fix the native image loader to not require 64K alignment

We may to tweak the file format so that we can get the page alignment, without the padding.

How are the native binary formats like ELF dealing with the 64k pages? Do they have the sections aligned at 64k boundaries as well?

@jkotas We objdumped an elf. The sections are only aligned to something like 8 byte boundaries. The VA of the section is incremented by a multiple of the maximum page size. I think for Arm64 is should be 16byte boundaries.

I took a look at "Visual Studio, Microsoft Portable Executable and Common Object File Format Specification Revision 9.3 – December 29, 2015" the following statement appears in the text.

If the SectionAlignment is less than the architecture’s page size, then FileAlignment must match SectionAlignment.

I think this would allow the PE to behave similarly to ELF.

OK I created PR #10959 with the ZapWriter Loader rework to prevent this bloat.

sdmaclea · 2017-04-11T15:55:21Z

src/zap/zapwriter.cpp

@@ -55,7 +55,7 @@ void ZapWriter::Initialize()
    m_FileAlignment = 0x200;
 }

-#define SECTION_ALIGNMENT   0x1000
+#define SECTION_ALIGNMENT   0x10000



Section alignment to match maximum supported page size.

sdmaclea · 2017-04-11T19:35:52Z

src/ToolBox/SOS/Strike/gcroot.cpp

 #else
-#define card_size ((size_t)(DT_OS_PAGE_SIZE/card_word_width))
+#define card_size ((size_t)(4096/card_word_width))
 #endif //_TARGET_WIN64_


It would be nice to stop duplicating this GC code here.

sdmaclea · 2017-04-11T19:36:27Z

src/ToolBox/SOS/Strike/platformspecific.h

@@ -190,6 +190,6 @@ struct DT_RTL_USER_PROCESS_PARAMETERS

 #endif // !FEATURE_PAL

-#define DT_OS_PAGE_SIZE   4096
+#define DT_OS_PAGE_SIZE   0x10000



There may be complications here for remote debugging with target having separate size as host.

Also this may need a different mechanism to determine the os page size dynamically.

sdmaclea · 2017-04-11T19:39:53Z

src/gc/gcpriv.h

 #else
-#define card_size ((size_t)(OS_PAGE_SIZE/card_word_width))
+#define card_size ((size_t)(4096/card_word_width))
 #endif // BIT64



These may be better converted to hard numbers. Since they are supposedly empirical.

#if defined (_TARGET_WIN64_) #define card_size 0x200 #else #define card_size 0x100 #endif

They are empirical, yes, but they are also tied to GetWriteWatch operating at page granularity. If we change these, we'll also need to change the software write watch tables and card bundle tables (cc @adityamandaleeka)

sdmaclea · 2017-04-11T19:42:30Z

src/gc/gc.cpp

@@ -6306,7 +6306,7 @@ void gc_heap::make_c_mark_list (uint8_t** arr)
 static const size_t card_bundle_word_width = 32;

 // How do we express the fact that 32 bits (card_word_width) is one uint32_t?
-static const size_t card_bundle_size = (size_t)(OS_PAGE_SIZE / (sizeof(uint32_t)*card_bundle_word_width));
+static const size_t card_bundle_size = (size_t)(4096 / (sizeof(uint32_t)*card_bundle_word_width));


It would be nice to have a name for this magic number. Suggestions?

GC_PAGE_SIZE

sdmaclea · 2017-04-11T19:52:10Z

@jkotas PTAL at OS_PAGE_SIZE --> 4K changes and provide further direction.

jkotas · 2017-04-11T20:39:11Z

cc @Maoni0 @adityamandaleeka @swgillespie for the GC changes

swgillespie · 2017-04-11T21:01:17Z

src/gc/gc.cpp

@@ -18786,13 +18786,13 @@ void gc_heap::fix_card_table ()
                    time_stop - time_start, tot_cycles);
 #endif //TIME_WRITE_WATCH

-            assert( ((card_size * card_word_width)&(OS_PAGE_SIZE-1))==0 );


On Windows, this really is the true operating system page size and not an arbitrary constant. The array of pointers that we receive from GetWriteWatch (used to track writes on the heap during concurrent GCs) are page-sized. When we're looping over this array, we're looking at single OS pages and setting cards for it.

I don't think that GetWriteWatch supports anything other than 4k pages, so it's probably fine to replace these uses with 4096, but perhaps we should assert somewhere that if we are using GetWriteWatch, OS_PAGE_SIZE is what we expect.

swgillespie · 2017-04-11T21:01:54Z

src/gc/gcpriv.h

 #else
-#define card_size ((size_t)(OS_PAGE_SIZE/card_word_width))
+#define card_size ((size_t)(4096/card_word_width))
 #endif // BIT64



They are empirical, yes, but they are also tied to GetWriteWatch operating at page granularity. If we change these, we'll also need to change the software write watch tables and card bundle tables (cc @adityamandaleeka)

sdmaclea · 2017-04-14T21:57:54Z

I am splitting this pull request into two PR's. Rather than force pushing, I am going to close this PR to preserve the conversation history.

The first request is #10959, the second is #10981

Draft 64K static support

864f227

dnfclas added the cla-already-signed label Apr 11, 2017

sdmaclea commented Apr 11, 2017

View reviewed changes

Use 64K page for GC

3d7aec5

sdmaclea commented Apr 11, 2017

View reviewed changes

jkotas requested a review from janvorli April 11, 2017 20:38

swgillespie reviewed Apr 11, 2017

View reviewed changes

BruceForstall added the * NO MERGE * The PR is not ready for merge yet (see discussion for detailed reasons) label Apr 12, 2017

sdmaclea closed this Apr 14, 2017

sdmaclea mentioned this pull request Apr 14, 2017

[Arm64/Unix] Add 64K page support #10981

Merged

sdmaclea deleted the WIP-64kPages branch April 27, 2017 21:58

karelz modified the milestone: 2.0.0 Aug 28, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Arm64/Unix] WIP-DoNotMerge-Draft 64K static support #10891

[Arm64/Unix] WIP-DoNotMerge-Draft 64K static support #10891

sdmaclea commented Apr 11, 2017

sdmaclea Apr 11, 2017

sdmaclea Apr 11, 2017

sdmaclea Apr 11, 2017

sdmaclea Apr 11, 2017

sdmaclea Apr 11, 2017

sdmaclea Apr 11, 2017

jkotas Apr 11, 2017

sdmaclea Apr 11, 2017

janvorli Apr 11, 2017

sdmaclea Apr 12, 2017

sdmaclea Apr 12, 2017

sdmaclea Apr 12, 2017

jkotas Apr 12, 2017

sdmaclea Apr 13, 2017 •

edited

Loading

sdmaclea Apr 13, 2017

sdmaclea Apr 11, 2017

sdmaclea Apr 11, 2017

sdmaclea Apr 11, 2017

sdmaclea Apr 11, 2017

swgillespie Apr 11, 2017

sdmaclea Apr 11, 2017

jkotas Apr 11, 2017

sdmaclea commented Apr 11, 2017

jkotas commented Apr 11, 2017

swgillespie Apr 11, 2017

swgillespie Apr 11, 2017

sdmaclea commented Apr 14, 2017

		@@ -4,6 +4,7 @@



		#define OS_PAGE_SIZE 0x1000
		#include "common.h"

[Arm64/Unix] WIP-DoNotMerge-Draft 64K static support #10891

[Arm64/Unix] WIP-DoNotMerge-Draft 64K static support #10891

Conversation

sdmaclea commented Apr 11, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sdmaclea Apr 13, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sdmaclea commented Apr 11, 2017

jkotas commented Apr 11, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sdmaclea commented Apr 14, 2017

sdmaclea Apr 13, 2017 •

edited

Loading