-
-
Notifications
You must be signed in to change notification settings - Fork 4.6k
[RFC] Preloading support #7777
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Comments
My first idea on the way this would work would be like the I don't think preloading all classes always is a good idea, that'll just blow up memory usage for no reason. This is also very cheap to generate much like the files autoloading it's simply dumping an array of files in preload.php so it can be built always no need to be optional like optimized autoloader. |
I see. That's a nice idea too!
I agree, it makes no sense to load all the classes if not all of them are used. However, as the developer of an app I would like to be able to optimize this myself so I don't think it makes sense to give the responsibility to define the classes that need to be preloaded to the developer of the library. Let's take the Symfony VarDumper component as an example: In 99.5% of all cases, this is used for debugging purposes only so it likely makes no sense to preload classes of it. But what if somebody builds an API service that uses this component to style some output? Preloading would make sense there then. I also wondered how much RAM usage we're talking about. So here are some stats for everybody:
Then I ran $classmap = include './../vendor/composer/autoload_classmap.php';
$classmap = array_unique($classmap); // Needed because we have multiple classes in some files
_preload($classmap); I reset the cache and visited the welcome page again: Memory usage increased to 27.74 MB (919 cached files). So yes, memory usage does increase but is it really that significant? I mean, if you enable preloading in the first place you're after good performance, right? Is it a problem then that your app uses a fair amount of RAM constantly? All I'm trying here is really to just throw in some numbers so we can weigh up the pros and cons.
Maybe there are more variants? 😄 |
Here's the numbers of a bigger project using ApiPlatform, Doctrine, Guzzle, Enqueue, Symfony Translator, Redis, Symfony Console etc.): Index page(docs endpoint) without preloading everything: 33.36 MB (15.29 MB net) |
So you need more then double amount of ram. In other words you can serve only less then half the users in comparison to non-preloaded with the same server |
Well, that's not a fair comparison. I called just one endpoint. I would need to call the other endpoints that use other components one after the other if we wanted to find out the percentage of "useless cached files". I think I'd get a lot closer to the 57.24 MB if I did that 😊 |
Still a lot of overhead. More then I would like to pay to call the feature usefull when loading all the things IMO loading everything only works for small apps |
I appreciate your enthusiasm, buuut I don't think any of this is composer's responsibility. I fully agree that it's most likely app specific what you want to preload, but there are two things to consider in my proposal:
Lastly regarding the option of preloading the whole classmap, that's a 3-line foreach loop that you can write as your own preload script loading all classes from composer's classmap if you are so inclined to waste memory ;) It could also be offered as a package So my position at least for now is to either do some very simple thing to facilitate things and "standardize" on a |
I tend to agree with @Seldaek here. The preloading-everything strategy is already straightforward once you generate a full classmap for the optimized autoloader (which you should do anyway if you care about performance, and you don't need preloading if you don't care). |
BTW: I'm not inclined to waste memory at all. It was just one variant that came to my mind and so I elaborated on it. I think it's good to consider multiple approaches, also for the people that read the issue later on. I'm perfectly fine with having the bad ones ruled out 😄 We could allow the |
@Toflar if it is root-only, why asking to put them in composer.json so that composer requires them in another file that you can then reference in your php.ini ? You could reference your own file directly in the php.ini. |
I know 😄 The only thing it would do is somewhat "standardizing" the way to do it. Nothing else 😊 |
I would think that creating levels for autoloading like logging levels would allow for some sort of "automated" control with the library providing the files/classes that would be appropriate for the level. An uber level like "all" could include all files including the files for libraries not supporting the levels. Sounds like something that might fit into the PHP-FIG realm of discussion as well. |
Some disorganized thoughts:
|
I totally agree that <?php
spl_autoload_register(function($name) {
include_once("$name.php");
});
use Foo; About what should be preloaded and not, keep in mind that to refresh the preloaded files you must restart php. I think the most typical use-case would be to preload your vendor but keep your application out of it so you can deploy changes to your application without a server restart. |
That's the same question I was asking myself. Because I understand it the way that you preload stuff that would be loaded into memory later on anyway. Just not on every request but shared. Which in turn means that "preloading all the things" doesn't effectively change the amount of memory used except for the percentage of classes that are not needed aka classes that are shipped with a library but never used within the context of the app. |
@Toflar your response seems to assume that all your requests are using all the classes from the codebase (so used by the project is the same than used by the request). For most projects, that's not the case. Many classes are used only by some specific requests rather than by all of them. |
If Composer does decide to do something, it should be root-only. Letting your dependencies decide what to preload is not helpful. |
Like @Crell and @Toflar, I was asking myself the same thing. I highly doubt that the memory used by preloading classes is copied to every single PHP process, I guess it's shared memory (to be verified), so preloading the whole stuff would "only" eat ~100MB, but every PHP process thereafter maybe eats less memory? I'm personally in favour of having composer.json generate a preload.php script that contains by default all the files from the I don't mind if there's a way to exclude some files explicitly, though, if these files are most likely never used in the average project. But I'm not sure whether any such file would actually belong to the
As far as I understand it:
The only issue I can think of, for your use case, is that
Then preloading is not for these projects, as explicitly mentioned in the RFC:
As such, here is how I would implement preload.php (my 2 cents):
This would be the kind of file that would be generated with no extra configuration. It's there, you can use it if you want, but you don't have to. Optionally, I would add a For those having multiple versions of an application / dependency: to reiterate, I would advocate to not use preloading at all. Or if you're feeling brave enough, fiddle with For all other users out there, just including the auto-generated preload.php will work like magic. |
Maybe we can get @dstogov to help us out to make the right decision for the PHP community here 😊 |
You've misunderstood @Toflar's comment about some classes only being used in some requests of the application vs multiple applications / multiple versions of applications. They're not the same thing at all. |
@teohhanhui I don't think I misunderstood @Toflar's comment, I was quoting @stof who was himself replying to @Toflar. To clarify my thoughts:
|
I think, preloading is a very new feature to immediately implement its support in composer. I tried preloading the whole frameworks (ZendFramework) and application specific preloading (getting the list of used PHP scripts through opcache_get_status() and generating a list of opcache_compile_file(...)). The second approach works better. Read about usage of Java Class Data Sharing. We implemented similar technology, and may borrow use cases. |
Thanks for jumping in, @dstogov! Could you please explain what you mean by works better? Did you get better performance? Did preloading all the classes take up too much memory? Did you have any issue? |
@dstogov Can you clarify the question above regarding the memory usage of a preloaded class? Viz, if I have 100 classes that are used on virtually every request anyway, and I then preload all of them, we know that's going to save CPU time. However, is it going to increase, decrease, or have no effect on memory usage? Similarly, if we preload 10 MB worth of code that is used only on a small fraction of requests, and there are 10 concurrent requests, have we now increased total memory usage by 10 MB (shared memory) or 100 MB (cost in each process)? |
@BenMorel The example from Platform.sh is not a class at all. It's a file that looks something like this: <?php
function stuff() {
$_ENV['db_name'] = $_ENV['dbname'];
}
stuff(); (Because the application wants an environment variable with one name and our system by default provides it with another. This is a very over-simplified example but it gets the idea across.) That file is then included by composer so it runs during autoload, before the application looks for its environment variables. There's nothing intrinsically wrong with that approach and it works quite well right now. My point is that such a file MUST NOT be preloaded, because then it won't actually run on subsequent requests and break the application. We don't need to do anything special here for it other than make sure that it doesn't get picked up and preloaded accidentally by whatever mechanism Composer ends up using. |
@Crell preloading such a file would have zero effect the way I understand it. As it doesn't declare a preloadable class it is ignored, and will be executed at runtime when included. |
@DarkGhostHunter As have been pointed out multiple times in this thread, just preload everything. No need for such unnecessary configurations that will most likely end up in preloading too little anyway, thereby defeating the whole purpose. |
I also agree with what @Crell said about making composer determine the autoload files. This can easily increase the complexity of Composer and should not be part of a dependency manager. The plugin I shamelessly self-plugged is also trying to generate a |
On a development machine it could work since there are no high RAM limitations like in a production machine. |
I really like the idea of the To me, the most sensible thing would be to develop a PHP package to analyse the code base of your project and generating the preload.php accordingly. This could be a composer plugin (or maybe something other that then gets integrated into a composer plugin) that will take any number of folders to check and try to resolve all the used classes and generates the file accordingly. This solution would have to rely on composer and the autoloader though, since there is no point in rewriting an autoloader just for this use case. This would come in handy for developers, since they would not have to care and think about what they would actually have to provide (and based on myself, not having to care about stuff is always awesome). Any thoughts? EDIT: I'm a bit concerned about the whole preloading stuff too, since having to restart my |
@CDRO maybe with the time, it will be changed and you will need to only reload |
I though FPM reload was a graceful restart, meaning workers (that have preload data in memory) are killed and restarted once the current request has been handled through. Restart would just kill the in-progress request and return an error to the client. |
If a reload does what @rask suggests, this would be indeed perfect, maybe @nikic has a better overview if it's like we expect it to be. @pjona you're right, if this is an issue, HA should already be implemented, on the other hand this can easily be solved with deployment windows too, where it is accepted that the application might be down for some short time. |
An FPM reload will not clear the preload state, you do need a full FPM restart. |
@nikic I see. FPM workers receive a baseline exec env from the process manager, which is what must be restarted for the workers to receive a new exec env properly? |
I'm not entirely sure it is possible to generate a useful list of stuff to preload without any execution stats, unless composer is somehow analysing the codebase and its real points of entry to see what's actually being used and what not. This is something a static analyser is probably better suited to do as some of the tooling required should already be there. I'm currently custom-creating the preload file from real world opcache stats after letting the app rI'm currently working on run for a few days in the wild (wild meaning automated traffic as the app is still in dev). This solution does work and can potentially be automated to some extent, but it'd be a custom job each time. I'm not convinced composer is the right tool for this particular job. |
On the subject of preloading everything, and taking benchmarks above such as they are, the extra memory used by opcache can be very problematic when you're running your apps on highly promiscuous environments with very tight memory constraints. For instance, kubernetes. Any one node can be sharing a meagre 4GB of ram with 15 or 20 pods which resource limits and requirements have been tightly adjusted. |
I think this is the preloading we are looking for: offer something basic, but let the developer expand on it. {
"preload": {
"entrypoint": "entrypoint.php",
"script": ["foo.php", "bar.php"],
"directories": ["examples", "foo/bar"],
"files": ["helpers.php"],
"ignore": ["src/foo.php", "src/bar.php"],
} This gives 100% flexibility on what to preload:
The procedurePreloading means editing
That will cycle every package for a Ideas? |
@DarkGhostHunter I'm new here and don't see under the hood of composer. But from the perspective of a mere user, this looks good to me and like something I could work with 👍 |
While my suggestion will allow for automatic preloading, there is still progress to be made on preloading only the "hot" files. There should be a way to save OPCache analytics about what files are hit the most, and push a part of the list based on memory constraints or percentage threshold. If composer could do part of that job, it would be awesome The later matters because you may preload a project with 1500 files, but you may get almost the same performance for 99% of requests with just 150 files. That you you could instance 10 more PHP instances instead of just one. |
I think everybody agrees that the most optimal way to generate the preloading is to gather information via opcache and load only the files needed for the project. IMHO, since most projects using composer will probably build their application on a deployment server and then push the app/website to the production server, But what if it could actually access this information? I could imagine the following solutions regarding these issues:
Would this be a viable solution? |
You are welcome to keep the discussion going here as a central point for people interested in the topic to coordinate. But just to be clear, I am fairly confident that in the near future we are not going to add anything to Composer relating to preloading. If in a year it turns out - after people have been playing with it - that there is something Composer is uniquely positioned to really help with, we can revisit. For now it seems to me much more like an application/deployment concern than a dependency management one. |
I think the problem here is we are trying to preload everything. What if we give the responsibility to package developers instead? They can declare classes/files that needs to be cached in the composer.json.
Package developers are responsible for these files. They should not have dependencies (or at least include it in preloading). Composer will detect these declared files and automatically create a preload script which we can optionally use. |
The problem is not preloading everything. The problem is to preload what is
useful and what's not, something that is out of the scope of Composer; a
good preloading list is made by Opcache stats and a memory limit, and has
to be manually injected into php.ini.
For that purpose alone I created a package that checks your most requested
files and creates a list containing these first in descending order.
Preloading all files is an irresponsibility when you consider that, after
certain point, adding more files to the preload list will have marginal
returns in performance.
El vie., 13 de dic. de 2019 23:53, kapitanluffy <notifications@github.com>
escribió:
… I think the problem here is we are trying to preload everything.
What if package developers can declare classes/files that needs to be
cached in the composer.json.
{
"preload": [
"/package/AbstractClassInterface.php",
"/package/AbstractClass.php",
"/package/helpers.php",
]
}
Package developers are responsible for these files. They should not have
dependencies (or at least include it in preloading). Composer will detect
these declared files and automatically create a preload script which we can
optionally use.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#7777?email_source=notifications&email_token=ABHHLF6ZSVZNW3DN6N4NYOLQYRDAXA5CNFSM4GCIOG4KYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEG3YFOY#issuecomment-565674683>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ABHHLF2GLDG5CBQ7NGUQRK3QYRDAXANCNFSM4GCIOG4A>
.
|
Trying to see this from a simple users perspective. I think I read trough all the comments here. Did I miss something or really nowhere this scenario was mentioned? A user is hosting on a shared web host or a server he hasn’t much control of. And the user is using any kind of software which is using composer. Let’s imagine in each package or on root level the preloading behavior is defined. The user even don’t know about it. Well, yes it’s true, there still is the php.ini step which prevents from an accidental activation of PHPs preloading behavior (if there is not such a thing like a ‘default link’ to composer/preload.php from PHP itself in future, which Toflar was mentioning before). But what if a user is activating it because he found out about it in any kind of documentation, but is not aware about the full result (good and bad) of it? How the user will be able to get the pre-loaded files out of its memory in its shared host? In my opinion activation of preloading should urgently be based on a strong opt-in. For instance by being required to explicitly install the required code within a separate package which is more complicated and prevent from ‚by accident activations‘. Neither Composer nor a package maintained should decide this for the user without it’s acknowledge. Isn’t it? Of course this doesn’t mean that a preloader should not use the Composer generated class map. |
@NickSdot You can't fix naiveness and irresponsibility with a Composer package. I agree, but the technique to properly make an optimal preload list is beyond Composer, so any point here apart from just seeing how the preload progresses in time is moot, imo. |
Closing as I don't think we will ever do anything here, and the discussion seems to have run its course. |
As a general preloading file seems to become a thing in PHP 7.4 (🎉) I think we should start the discussion on how this could be implemented in Composer. I'm willing to work on a PR for that but there are a few things to be clarified first.
Here are just a few thoughts that come to my mind when thinking about the implementation:
vendor/preload.php
?composer dump-autoload -o -p
(-p
would then also generate thepreload.php
)I'm sure there's more to be considered and discussed. There are some very smart cookies in our community, so let's try to lay out the best solution together first and only then start coding 😊
The text was updated successfully, but these errors were encountered: