Thanks Maxim. I did not notice that check of a->elts. I understand to not to fix the memory optimization as you do not want to bring in pool intelligence into array utility. Still, I think we used pool data to some extend. If there is advantage with memory optimization I think we should think about handling it.
Thanks, -Ravi. On Thu, Jan 16, 2014 at 8:04 PM, Maxim Dounin <[email protected]> wrote: > Hello! > > On Thu, Jan 16, 2014 at 07:16:50PM -0800, Ravi Chunduru wrote: > > > Hi Maxim, > > I understand on array overflow nginx creates a new memory block. That > is > > perfectly alright. > > > > Say I have a Pool and array is allocated from the first memory block and > it > > happend such a way that array elements end at pool->d.last. > > > > Now, say the pool is used for some other purposes such a way that > > pool->current is now pointing to a different memory block say '2' > > > > And if we want to allocate a few more array elements, nginx has to use > > second memory block. Now the elements are moved to second memory block. > > > > At this stage, if any new element is requested that results in overflow, > > nginx does the below checks > > > > if ((u_char *) a->elts + size == p->d.last > > > > && p->d.last + a->size <= p->d.end) > > > > In the above code, p->d.last was actually pointing to the end of first > > memory block but not second memory block. Hence *even though there is > > memory available in second memory block* it will go ahead and create a > new > > memory block. This will repeat on each overflow. > > Yes, I understand what you are talking about. As array never > knows from which exactly block the memory was allocated, and > doesn't want to dig into pool internals, it only checks an obvious > case - if the memory was allocated from the first block, and if > there is a room there. > > > And, the code in ngx_array_destroy() will actually set the > > pointer(p->d.last) wrongly once there is a overflow. This is a critical > > issue. > > First memory block would have say 'n' elements but after overflow number > of > > elements become 2 times of n. > > Lets say after second overflow, I destroyed the array, then p->d.last > will > > be set backwards by 2 times in the first memory block. But, in actuality > it > > was size 'n'. > > The code in ngx_array_destroy() does the following: > > if ((u_char *) a->elts + a->size * a->nalloc == p->d.last) { > p->d.last -= a->size * a->nalloc; > } > > If a memory was allocated from another block, the "a->elts + ... > == p->d.last" check will fail and p->d.last will not be moved. > > > > > Nginx never faces that situation because, once a memory block is set to > > 'failed', it wont be used for allocation any more. But, if the 'failed' > > count is less than 4 then we may have issue and also pool destroy may > have > > potential issues. > > > > Sorry for long email, but I wanted to explain that in detail. > > > > Thanks, > > -Ravi. > > > > > > > > On Thu, Jan 16, 2014 at 6:40 PM, Maxim Dounin <[email protected]> > wrote: > > > > > Hello! > > > > > > On Thu, Jan 16, 2014 at 06:22:58PM -0800, Ravi Chunduru wrote: > > > > > > > Hi Nginx experts, > > > > Thanks for the prompt reply to my earlier email on ngx_reset_pool() > > > > > > > > Now, I am looking into ngx_array.c. I found an issue > ngx_array_push(). > > > Here > > > > are the details. > > > > nginx will check if number of elements is equal to capacity of the > array. > > > > If there is no space in the memory block, it allocates a new memory > block > > > > with twice the size of array and copies over the elements. So far so > > > good. > > > > Assume that pool utility got entirely new memory block then a->pool > is > > > not > > > > updated with that of 'pool->current'. > > > > > > > > I got an assumption from the code that a->pool is always the memory > block > > > > that has the array elements by seeing the code in ngx_array_push(), > > > > ngx_array_push_n() or ngx_array_destroy() where checks were always > done > > > > with pool pointer in array. > > > > > > > > Functionalities issues would come up once there is an array > overflow. I > > > > think for every new push of element after first crossing/overflow of > the > > > > capacity, nginx will keep on creating new array. Thus it results in > > > wastage > > > > of memory. > > > > > > > > Please let me know if its a issue or correct my understanding. > > > > > > That's expected behaviour. Arrays are implemented in a way that > > > allocates additional memory on overflows, and it's expected to > > > happen. There is a ngx_list_t structure to be used if such > > > additional memory allocations are undesired. Optimization of > > > allocations which uses pool internals is just an optimization and > > > it's not expected to always succeed. > > > > > > -- > > > Maxim Dounin > > > http://nginx.org/ > > > > > > _______________________________________________ > > > nginx-devel mailing list > > > [email protected] > > > http://mailman.nginx.org/mailman/listinfo/nginx-devel > > > > > > > > > > > -- > > Ravi > > > _______________________________________________ > > nginx-devel mailing list > > [email protected] > > http://mailman.nginx.org/mailman/listinfo/nginx-devel > > > -- > Maxim Dounin > http://nginx.org/ > > _______________________________________________ > nginx-devel mailing list > [email protected] > http://mailman.nginx.org/mailman/listinfo/nginx-devel > -- Ravi
_______________________________________________ nginx-devel mailing list [email protected] http://mailman.nginx.org/mailman/listinfo/nginx-devel
