Re: cvs: pecl /apc apc_main.c

[email protected] (Brian Shire) Wed, 29 Aug 2007 10:45:19 -0700
Newsgroups php.apc.dev
Message-ID <[email protected]>
Hey Gopal,

On Aug 29, 2007, at 1:34 AM, Gopal V wrote:

> Brian Shire wrote:
>
>>   Changing cache entry mtime when stat=0 to match the last known  
>> file mtime.    # Yell at me if this breaks anyone's expected  
>> functionality, or if there's a cleaner way to do this.
>
> Doesn't this completely make apc.stat = 0 completely pointless by
>

This code should cause it to stat() on the first request that's going  
to insert it into the cache.  After this, the stat=0 is still in  
affect and it should be completely bypassed on subsequent requests as  
the execution path is different.  The duplicate stat on the initial  
request is an unfortunate side-affect of doing it this way because  
I'm unable to change the make_file_key to do this stat() only on the  
initial insertion request because we don't know this information at  
that point.

I re-tested this just now by placing a warning message in this block  
of code, and I only see it happening on the initial cache insertion,  
subsequent requests are silent.


>
> Btw, I'm sort of planning to kill apc for php4 with the next release.
>

Sounds fine by me, anyone who is running php4 and apc is kindof  
defeating the point due to the large speed/memory gains by running  
php5. (at least that's been my experience).  It also seems like the  
internals list has also decided to finalize this, so it will be nice  
to clean up the code of all the conditionals we have to deal with  :-).


> So I've been resisting changes which might break too many things,
> because I sort of want to rewrite the entire apc_compile.c etc ...
>
> http://t3.dotgnu.info/blog/php/apc-yak-shaving.html
>

I recall reading this a while ago as I stuck your blog on my rss  
feed, I think it's a great idea.  My only concern immediately is how  
it will affect my outstanding patch for doing binary dumps to files,  
as I rely on these copy* functions to pull the data from the cache.   
I can submit the full patch to the list soon, to see what everyone's  
opinions are and if there's other ways to go about this.  It just may  
create more dependencies that we'll have to deal with.  Happy to help  
fixup that part of the code while you make changes too though or  
perhaps sit on it for a while?

I was also thinking today that maybe we should mark --enable- 
pthreadmutex as not "EXPERIMENTAL" any longer.  I think I have one  
outstanding issue with linking to look at on this, but the code  
itself should be pretty stable (I've been running it for a while).   
I'm going to take a look at spin locks here soon too to see if I can  
reproduce the problems Rasmus/Ilia have reported.


> If you don't mind, can you revert this particular patch?
>

Let me know what you think about this, and if it's still a problem.   
Perhaps there is another place we could be doing this too?


> Cheers,
> Gopal
> -- 
> He who foresees calamities suffers them twice over.

And sometimes thrice when they have to go and fix the calamity (read:  
"bad design").


-shire