Jump to:
| Project: | High-performance JavaScript callback handler |
| Version: | 7.x-1.x-dev |
| Component: | Miscellaneous |
| Category: | task |
| Priority: | normal |
| Assigned: | Unassigned |
| Status: | closed (fixed) |
Issue Summary
I would like to move our general reflections on the further development regarding this basic question here. Michiel has already proposed a quite good idea with auto-generating a customized js.php - I like this approach a lot. Nevertheless, there are some pros and cons, as always are, here are the ones I see.
Pros:
- I as a module dev like that a lot.
- I as someone into simple and convenient interfaces like it even more.
Cons:
- Not every site builder is a module dev or just wants to be one. Those who just download, say, admin menu und want to speed it up, will not necessarily know which options to set and why.
- What if module dev A addresses the most basic js.php (call it "the original one") while module dev B relies on the site builder to activate feature x and y - we would at least still have to make sure, that all optional feature constellations are cross-compatible.
However, I would still like to convince you of the "two basic callbacks" idea. Let's call them "p-callback" for the classic one ("performance") and "x-callback" for the extended one. For compatibility, the first one stays named "js/", the latter one "jsx/" (as I proposed in the other thread).
We could still use all the - really good! - extensions we just made, but would put them in the x-callback and still add cool new features without risking anything. We would not even hurt someone, as hardly any user of our module would already have launched own releases with new functions basing on the most recent new features. (Except for neilnz, probably, but he is for sure following our issues ;))
Also, I would suggest that, if we stick to generating an (even custom!) js.php, we should not do it half way (which is: downloading, saving, uploading) but simply saving the js.php directly to the Drupal root. I have this solution in chatblock, where another php file is needed in the root dir, and that works fine. We can also try the same with the .htaccess modifications, I have a rough idea of how we can do this - if we decide so - and would try to implement both features.
Appreciating your opinions!
Comments
#1
I want to avoid the option of having two separate callbacks as long as possible because that in in itself will cause other, unforeseen, problems.
I really think the generated js.php file is the way to go, I solves all the problems we have and only creates a few minor ones.
I agree on your point about it always behaving the same way for all features, regardless of which ones are enabled. I suggest making a simpletest for this to verify this as good as possible. I don't know yet how I will handle this as the web test will require write permissions to the root of the Drupal installation, not a good idea. This neatly hooks into why I offered the js.php as a download instead of writing it to the root. I really don't want anything to be able to write and actually write to my Drupal root folder. This would have some serious security implications.
As for the note on site builders, you're right. They will not understand how this would work, but then again neither would they if we stick to the original version or two separate callbacks. This is the main reason why the second sentence on the project page is that the module is only for developers :) I really think we can never have this module friendly for site builders and we shouldn't make any effort in that direction. It's a dead end for now..
About the dependencies in other modules, I see that it might be a problem. But these other modules can easily check in the requirements hook to see if the variables for the js module are set correctly.
#2
Hi,
I think we should clarify the terms because I feel we mismatch here. The dead end you describe isn't really one, IMHO. So:
AFAIK, in Drupal terms "site builder" means "user who is able to click himself his own Drupal site, add modules and configure a theme but not much more", while "dev" is the one preparing all that clickywicky "ready to eat" core, module and theme stuff (like we do). Sure: We two and the people probably following our issues are site builders as well, so we know dealing with deeper levels of Drupal, but the opposite way is rather a rare case.
In consequence, my understanding of "this is a developer module" is actually not that this module is (or should be) targeting only module devs which also build sites, but also site builders who want to use modules which are provided by folks like us. Let's take my upcoming "comments and quotes suite" or my chat module as an example. I recommend JS not because I want the site builders to do some hacking or even just cryptic configurations theirselves, but because I assume they want everything running nice and quick. Perhaps the term "API module" fits better anyway?
Let's continue this to the config options: While the developer of a contrib module is not necessarily the one building the site, take one site builder and two module devs with each one module. Module A requires setting X=1 and Y=0. Module B requires both X and Y to be =1. What do you want to do? Who is to set the site config options? Dev A? Dev B? None of them actually is, but the site builder is. And his only real choices are: Either drop one of both modules or hack into the modules so their requirements change. Of course you could no start offering configurations per module. But why would you? The module developer is the one to know which settings are required, and thus this has to happen per module and directly in each module.
This is why we should clearly stick to a consistent API with hardly any site builder settings (except for "show .htaccess" and "generate js.php" of course). Other than you say, the site builder will not get touched by two callbacks at all, because this is all up to each module. Dev A can say "ok, i use the p callback". While B will configure his hook_js() to use the "x callback". Actually this is what happend until now, that is what hook_js() was created for. Meanwhile, the site builder smiles as everything works without configuration that he doesn't understand anyhow. Don't you please get me wrong, I really like your config page and the template idea, and I will most likely clone it for some other projects of mine. But nevertheless I am convinced that we should leave this module as a transparent API working horse.
I mean, maybe I really got this module wrong from the beginning and it actually was always intended to only serve a small group of folks who are both into developing and site building, but I as a contrib module dev would then re-consider using it if I want my modules to provide maximum performance and instead go the ugly way of creating a module_js.php for any single project to be really sure that there is no interference like the ones I tried to describe above. And this is somehow the least thing I would want to do. (Hum. I made hundreds of words and still am not sure if I managed to bring up my point. So I'll stop here.)
However, even if we really wanted to leave developer related settings to the site builder, we can still try with two callbacks. Internally, we could simply set a variable
<?php$extended = array_shift($args) == 'jsx';
?>
For the security argument, I can partly agree. .htaccess may really be to hot to handle (although, take a look at how boost does it). But I really don't see implications if we add a file to the Drupal root that would not exist otherwise and which we require the site builder to place there anyhow? Drupal root is not write protected by design, and there is a permission system that limits access to this function (at least in our case). Also, the js.php it may be automatically deleted on module uninstall and/or disable. So can you be more specific on that or is it rather a "people shouldn't do that because they shouldn't" feeling? Also, we could make direct writing to root a second choice aside downloading, so no one has to if he does not want to.
To stop that epic comment and do something practical: I will just create a 7.x-2.x branch (yet without release) and commit my ideas, so you can see how I think it might work and if or not it is really unbearable for you.
Simpletest is a good idea anyhow, I really should start to understand all that myself. So I will like to see your findings and test routines.
#3
The options in the configuration form only progressively add features to the js.php, one option does not make the other not work. So it wouldn't make sense for a module not to be able to handle any option beeing on. That is what we were trying to achieve with the latest patches for the page arguments thing.
In the current form, with the readme having a very clear explanation and the configuration page stating what you should do I think the module is already friendly enough for any non developer to use.
I always make extra sure that only the files folder in my drupal I stall is writable, an important security measure. Only there users should be allowed to change anything, nowhere else. That boost actually writes to the .htacces is not ideal in my view. But, maybe, we can find some compromise in this. Maybe we can try to write the js.phpto the root after saving the configuration form and, if it fails notify that the user should download the file and place itin the root. This would be the best compromise in my opinion.
I'm not of fan of starting a separate branch yet, as the 7.1 branch is not even fully developed. Let's focus on finishing this branch first before splitting up the development. I'm sure we can find a good solution with the current developments..
#4
No branch split up, but posting all suggested changes here would bloat the comment. Thus I will just create it so you can have a look at it. I think my current idea on how to handle the js.php autogeneration should also satisfy your security concerns. And yes, we will find a good way for all positions in just one branch, that's what I think, too and what we should keep on aiming for :-)
Right now, I find other things disturbing. I am not sure where the fault ist, because I tested your first draft (in the clone module, that is) with chatblock and it worked. Now it does no more, neither the first one nor the other one. Same with other modules. I will open an issue and kindly ask you to do a quick test and counter check whether I do overlook something or there is something else. If you will. Thx!Fixed. But say, did you change the default bootstrap level away from _LANGUAGE? I don't remember this. However.#5
Just one more thing back to "site builder config" vs. "api config": Even if all levels and options stay compatible, all(!) callbacks would perform as slow as the slowest one configured. If only one module requires all four options, even the simplest barebone "GET?iamstillhere=1" request would have to run through all the functions the most expensive callback needs.
The problem with the centralized site builder configuration is really that you take away the possibility for module devs to design their modules as CPU saving as possible as soon as only one other module requires more work. We really really should not do that. However it is really not that I don't like the new features. Au contraire, especially the access check is a nice thing to have. But only if one really needs it, and this isn't decidable per site, but per module :)
Perhaps there is a third way we are not yet aware of which allows us to keep one callback, all the new features but still configurable via hook_js. Just give me some time now to manage this Xmas thing. I'll use my 2.x "playground" to find a solution, please give it a chance once you see it :)
Edit: I already got an idea for that third way. It is as ridiculously simple as handy and will be able to take any future extensions with nearly no performance loss and with all setup power back from site builder to modules. Really looking forward to how you'll like it :)
#6
Done. If you want to take a look, just check out the 2.x. As I promised, it covers all features, needs almost no additional code, solves the "site wide performance impact" issue (as I find it: nifty) and should really satisfy all desires. Due to the logic behind the "complex paths" option, it would not work with just one callback - but in the end, there is no duplicate effort for any dev. I tried to clarify this in the readme (deeply revised). It is really none of a deal at all. Oh, and I also added the js autogeneration respecting your security concerns :)
#7
I looked at it but the new version doesn't make much sense. It replaces a few if statements with some other, it doesn't really solve the performance/feature issue. I have really looked deeply into the performance and the impact on all features and came to the following conclusions:
The page arguments, access callback and file options costs 0.0000028610229492188 seconds if you don't use them. This is benchmarked on a very slow, non apc, xdebug enabled server, ie: the situation could not be slower. Basic shared hosts would be a lot faster than this.
The complex paths feature costs a little more, the difference here is 0.000342845916748. This is such an incredible low amount it's just not worth spending any more time over. We've created a fast car instead of a slow one to cover a 1000 kilometer trip with the js module. Making these features optional only makes this trip 5 meters shorter. Not the optimization we're looking for. On a bare bones Drupal the difference between standard Drupal and js is this:
Standard: 0.23339581489563s
Js: 0.066429138183594
This is the gain we're looking for and this difference is only going to grow with more modules enabled. All the features brings this module closer to Drupal and makes it better understandable and implementable by others. The features makes sure it's easier and neater implementable in a lot of different cases, not just our own, above all it only adds 0.5% time, that's a very low and unnoticeable number. I still agree that we should be careful with adding new features and make sure the module will always be about performance. We shouldn't however focus on micro optimization, that's always the wrong way to go.
I have committed the change that removes the configurable form and updated the readme to include all features. I'll tag this version as beta2 soon.
#8
#9
I don't completely get it, you have now removed all your extensions? I have only a small, unhandy screen right now (reading larger diffs is no fun with that, I'll try asap)? If so, why that? Leaving the config form off meets my request, of course, but supporting site builders with the js.php and/or .htaccess was just fine?! Or am I mistaking something here? :)
Regarding micro features: Yes, fine for me. Let's do it YAGNI-style ;)
#10
No, as I said in my previous comment, all the features are in beta2. I explained that the performance loss was almost zero when you're not using them and it's fully backwards compatible.
#11
Hi, ok, now I had the chance to see the beta-2 on a normal screen.
Regarding your performance statements, I think it is really ok now. Your tests convince me.
But what about the js.php download/auto-generation?
#12
As there are no different js.php versions anymore I removed the generate feature and reverted to the 'copy to root' solution, as described in the readme. It seemed a bit overkill to manage this via an interface with all the problems that it brings..
#13
I see.
#14
Automatically closed -- issue fixed for 2 weeks with no activity.