On Thu, Oct 1, 2015 at 11:54 AM, Matt Ficken <themattfic...@gmail.com> wrote:
> >> And what wincache does. It is slower but the request is served. >> >> >> WinCache (file cache) if it can't reattach, creates a new shared mem file > for just that process: see > http://svn.php.net/viewvc/pecl/wincache/trunk/wincache_filemap.c?revision=336846&view=markup > Line 1122. > I'm not sure how WinCache works, but opcache already implements most necessary functionality. Actually we should just: 1) If process can't map SHM - lets map it into different address and disable default SHM (ZCG(accel_directives.file_cache_only)=1) 2) extended file_cache may look into improperly mapped segment before reading it from file. But ff course this is more difficult. > > Yes, ideally it would be position independent, maybe using based pointers > for the structs in zend_smm_shared_globals, zend_accel_shared_globals, > etc.... > > I agree my bugfix is suboptimal but its the least complex option to get it > fixed for users in 7, so their requests get served. > > > My bugfix would increase memory usage, but so does a 2nd WinCache(or 3rd). > But at least if the 2nd OpCache is shared (the same procedure as if no > OpCache existed) its less likely to create a 3rd OpCache. For me at least, > often several processes in a row have this issue, so could have a 3rd or > 4th OpCache or WinCache(even more memory), possibly very quickly. > Your proposal is better than WinCache. > > Creating a 2nd OpCache, if its shared or per-process (like WinCache) seems > to be the simpler workaround. The trade-off is memory(especially if > per-process), time to recompile scripts, and sometimes breaking > opcache_reset(), opcache_invalidate() and stat cache pages(shared or > per-process OpCache). > This may break application logic. In worst case different SHM may keep different versons of the same script... > > Using the file cache should be better(won't slow down to recompile > scripts). But would still either require changing > zend_shared_alloc_startup() and other places because create_segments() > would still be returning ALLOC_FAILURE, OR creating a 2nd shared or > per-process OpCache anyway. Is this likely to be done? That would be great, > but it seems less likely because of the complexity. > This may be changed in some way. > > Up to 6 OpCaches(5 backups) may be way too many (based off map_retries, 25 > may be too many). My testing only ever gets a 2nd OpCache(never got a 3rd, > etc...). > 6 may be not enough anyway. If we have a limit it's going to be exceeded. MS will improve their ASLR once again... :) > There should be a workaround/suboptimal fix at least until someday when > the optimal solution is done, so requests get served. > In my experience, temporary incomplete solutions are almost never replaced with general ones. New and new incomplete fixes are added on top instead. Anyway Anatol, if you have time, can you please play with patch and give your opinion. Thanks. Dmitry. > > > @@ -74,18 +74,18 @@ static void zend_win_error_message(int type, char > *msg, int err) > zend_accel_error(type, msg); > } > -static char *create_name_with_username(char *name) > +static char *create_name_with_username(char *name, int num) > { > static char newname[MAXPATHLEN + UNLEN + 4]; > char uname[UNLEN + 1]; > DWORD unsize = UNLEN; > GetUserName(uname, &unsize); > - snprintf(newname, sizeof(newname) - 1, "%s@%s", name, uname); > + snprintf(newname, sizeof(newname) - 1, "%s@%s%d", name, uname, num); > return newname; > } > -static char *get_mmap_base_file(void) > +static char *get_mmap_base_file(int num) > { > static char windir[MAXPATHLEN+UNLEN + 3 + sizeof("\\\\@")]; > char uname[UNLEN + 1]; > @@ -95,13 +95,13 @@ static char *get_mmap_base_file(void) > GetTempPath(MAXPATHLEN, windir); > GetUserName(uname, &unsize); > l = strlen(windir); > - snprintf(windir + l, sizeof(windir) - l - 1, "\\%s@%s", > ACCEL_FILEMAP_BASE, uname); > + snprintf(windir + l, sizeof(windir) - l - 1, "\\%s@%s", > ACCEL_FILEMAP_BASE, uname, num); > return windir; > } > void zend_shared_alloc_create_lock(void) > { > - memory_mutex = CreateMutex(NULL, FALSE, > create_name_with_username(ACCEL_MUTEX_NAME)); > + memory_mutex = CreateMutex(NULL, FALSE, > create_name_with_username(ACCEL_MUTEX_NAME, 0)); > if (!memory_mutex) { > zend_accel_error(ACCEL_LOG_FATAL, "Cannot create mutex"); > return; > @@ -123,11 +123,11 @@ void zend_shared_alloc_unlock_win32(void) > ReleaseMutex(memory_mutex); > } > -static int zend_shared_alloc_reattach(size_t requested_size, char > **error_in) > +static int zend_shared_alloc_reattach(size_t requested_size, int num, > char **error_in) > { > int err; > void *wanted_mapping_base; > - char *mmap_base_file = get_mmap_base_file(); > + char *mmap_base_file = get_mmap_base_file(num); > FILE *fp = fopen(mmap_base_file, "r"); > MEMORY_BASIC_INFORMATION info; > @@ -194,24 +194,18 @@ static int create_segments(size_t requested_size, > zend_shared_segment ***shared_ > can be called before the child process is killed. In this case, the map > will fail > and we have to sleep some time (until the child releases the mapping > object) and retry.*/ > do { > - memfile = OpenFileMapping(FILE_MAP_WRITE, 0, > create_name_with_username(ACCEL_FILEMAP_NAME)); > + memfile = OpenFileMapping(FILE_MAP_WRITE, 0, > create_name_with_username(ACCEL_FILEMAP_NAME, map_retries>>2)); > err = GetLastError(); > if (memfile == NULL) { > break; > } > - ret = zend_shared_alloc_reattach(requested_size, error_in); > + ret = zend_shared_alloc_reattach(requested_size, map_retries>>2, > error_in); > err = GetLastError(); > if (ret == ALLOC_FAIL_MAPPING) { > /* Mapping failed, wait for mapping object to get freed and retry */ > @@ -241,7 +235,7 @@ static int create_segments(size_t requested_size, > zend_shared_segment ***shared_ > (*shared_segments_p)[0] = shared_segment; > memfile = CreateFileMapping(INVALID_HANDLE_VALUE, NULL, PAGE_READWRITE, 0, > requested_size, > - create_name_with_username(ACCEL_FILEMAP_NAME)); > + create_name_with_username(ACCEL_FILEMAP_NAME, map_retries>>2)); > err = GetLastError(); > if (memfile == NULL) { > zend_shared_alloc_unlock_win32(); > @@ -306,7 +300,7 @@ static int create_segments(size_t requested_size, > zend_shared_segment ***shared_ > *error_in = "MapViewOfFile"; > return ALLOC_FAILURE; > } else { > - char *mmap_base_file = get_mmap_base_file(); > + char *mmap_base_file = get_mmap_base_file(map_retries>>2); > FILE *fp = fopen(mmap_base_file, "w"); > err = GetLastError(); > if (!fp) { > > > >
