Re: [APC-DEV] Re: cvs: pecl /apc apc_main.c

[email protected] (Gopal V) Thu, 30 Aug 2007 13:27:14 +0530
Newsgroups php.apc.dev
Message-ID <[email protected]>
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
...
> 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.

My bad. I read the code once, ran it and saw stat() in my strace.

> 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).  

There are people still stuck on php4 land, but they need to move out
soon. Whatever gives them enough of a carrot ... :)

> 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. 

The copy* functions are going to be split into copy_in* and copy_out*
and most probably all you need will be in copy_in*.

> 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've sort of been slightly concerned about putting up something like
that into APC HEAD (people distributing binary php files).

I wouldn't mind a public patch to let me see how you do it, but
pushing into mainline might be a *real* pain to maintain into the
future.

> 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.

Yup, it'd be good to go off fcntl land for locks.

> 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?

Cache insert could be a place to do this (outside the locked section),
rather than doing it in apc_main.c.

But as of now, I'd rather not worry too much about design in that
codebase - so, leave the patch in.

Cheers,
Gopal
--
I'm just very selective about the reality I choose to accept.
               --- Calvin