Download & Extend

_hierarchical_select_form_has_hierarchical_select() is expensive (and called from hook_form_alter())

Project:Hierarchical Select
Version:6.x-3.x-dev
Component:Code
Category:task
Priority:normal
Assigned:Unassigned
Status:closed (duplicate)

Issue Summary

After running some profiling on my admin/build/block page, as it is very slow), it has come to my attention that drupal's element_children() is the 12th most time consuming function that is called on that page, it is called 3832 times, 1060 times of which are called from hs's form_alter, which take a total of 81,780 microseconds to complete. Is there any way to limit what is done in form_alter? or at least, limit where it runs. There are definitely going to be no hs elements on the blocks admin page!

Comments

#1

Title:form_alter is expensive» _hierarchical_select_form_has_hierarchical_select() is expensive (and called from hook_form_alter())
Category:bug report» task
Status:active» postponed (maintainer needs more info)

Very interesting!

I'm doing a hook_form_alter() to find #type = hierarchical_select form items (through the _hierarchical_select_form_has_hierarchical_select() function). I need to do that to set my #after_build callback, which cannot be set for an element, so that's why I have to do it this way.
The reason I need #after_build is:
- D5: D5 has no form cache, so I had to store $form['#parameters'] + the file in which the form definition lives, so that AHAH callbacks can work
- D6: D6 does have a form cache, but it also needs to work when $form['#cache'] is disabled, which happens for example in the case of the Views exposed filters form. So I can't rely on Drupal's own form cache: I still have to store $form['#parameters'] and the file myself.

That's why it's necessary. I realize that it's not efficient, but that's not my fault, it's Forms API's fault. :( Unfortunately the Forms API is very lacking from many perspectives and this is one of them: it is *not* designed with support for AHAH-powered form elements in mind.

If you can find a more efficient method for detecting a HS form item in the form (that's the expensive part, not the caching of $form['#parameters'] and the file), I'll happily commit the patch.

#2

Well, the only part of hs I am using is the taxonomy module, and I only have it enabled for a couple of node types. So in theory, form_alter would only ever need to be run on node forms for those content types. I understand that some of the hs modules could potentially be in use in a lot more places, but perhaps you could move the form_altering to the submodules rather than running the same generic code for all. That way, people like me who are only using a small part of the module do not get hit so bad.

#3

I *really* don't want to duplicate that code in every module that implements it. That'll result in lots of "WTF is this doing here" moments.

Anything else you can come up with? :)

#4

I'll think about this some more soon. Have you got any ideas yet?

#5

Status:postponed (maintainer needs more info)» closed (duplicate)

I think the only way around this is by no longer requiring HS' form cache. This might be possible by passing enough data via the Drupal path, i.e. hierarchical_select_json/%/%/% instead of just hierarchical_select_json.

Hence, let's put more thought into that at #528156: Performance: Remove Hierarchical Select's form cache and don't reconstruct the entire form anymore. Marking this as a duplicate.