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

Reduce calls to should_disable #166

Closed
jvoisin opened this Issue Mar 9, 2018 · 2 comments

Comments

2 participants
@jvoisin
Collaborator

jvoisin commented Mar 9, 2018

Currently, this function (along with its callee get_complete_funtion_path) is our main bottleneck. We shouldn't call it twice for "native" functions.

Ideally, we should split the list of functions to hook into two lists: one for functions to hook at runtime, and the other for functions that we manager to hook at loading time. The later could even be a hashtable or lists, but maybe it's a bit overkill, I don't know.

Or we could, for each "native" function, put the names of the ones we're hooking into a hashtable, so when we're entering the hook of one, we can simply check into the hashtable if it's here, and walk the linked-list if it is. If we're doing this, we could split the functions into several lists.

@jvoisin jvoisin added this to the 0.4 Oliphant Chuckerbutty milestone Mar 9, 2018

@jvoisin jvoisin self-assigned this Mar 9, 2018

xXx-caillou-xXx added a commit that referenced this issue Jul 6, 2018

@xXx-caillou-xXx

This comment has been minimized.

Collaborator

xXx-caillou-xXx commented Jul 12, 2018

I did some benchmarks using joomla's phpunit on the #188 . It seems to be a bit faster.
I used 24 rules from the default.rules file and added X line of rules with random names.

number of rules time on should_be_faster time on master
25 0m51.645s 0m50.151s
50 0m51.748s 0m52.506s
100 0m51.750s 0m57.207
500 0m51.963s 1m41.236s
1000 0m53.306 2m55.564s
5000 0m52.187s 15m46.462s
  • without snuffleupagus: 0m41.298s
  • with snuffleupagus and every functions hooked at startup (meaning zero hooked at runtime): 0m41.736s
@jvoisin

This comment has been minimized.

Collaborator

jvoisin commented Jul 13, 2018

Done in 7963580

@jvoisin jvoisin closed this Jul 13, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment