Originally brought up by Joel_Pittet and suggestions made by DamienMcKenna

Performance optimization for Views

Problem/Motivation

isset() is much faster than array_key_exists().

Proposed resolution

Use isset() Use both with a shortcut || after isset()

Here is the D7 related issue:
#2760419: get_option() micro optimization

CommentFileSizeAuthor
#7 D8-get_option_micro-2903843-7.patch638 bytesjoseph.olstad
#2 D8-get_option_micro-2903843-1.patch593 bytesjoseph.olstad

Comments

joseph.olstad created an issue. See original summary.

joseph.olstad’s picture

joseph.olstad
Primary language French
commented
StatusFileSize
new D8-get_option_micro-2903843-1.patch593 bytes

patch

joseph.olstad’s picture

joseph.olstad
Primary language French
commented
Parent issue: » #2760419: get_option() micro optimization
joseph.olstad’s picture

joseph.olstad
Primary language French
commented
Issue summary: View changes
joseph.olstad’s picture

joseph.olstad
Primary language French
commented
Issue summary: View changes
joseph.olstad’s picture

joseph.olstad
Primary language French
commented

patch to follow
see
#2760419-19: get_option() micro optimization

joseph.olstad’s picture

joseph.olstad
Primary language French
commented
StatusFileSize
new D8-get_option_micro-2903843-7.patch638 bytes

Joel_Pittets patch is the way to go.

we need to check that the array_key_exists because isset when the array element value is null would return false (meaning the test would fail) but we want to know either way.

So back to Joel_Pittets patch. Otherwise we risk breaking something.

According to Joel_Pittets performance analysis this gives a 100x improvement in performance , put into relative terms it is a 3ms improvement!

dawehner’s picture

dawehner
Primary language German
commented
Status: Needs review » Reviewed & tested by the community

Its certainly not a bad move, given that this is in an uncached scenario not the fastest piece of code.

joseph.olstad’s picture

joseph.olstad
Primary language French
commented
Issue tags: +Performance

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 7: D8-get_option_micro-2903843-7.patch, failed testing. View results

joseph.olstad’s picture

joseph.olstad
Primary language French
commented
Status: Needs work » Reviewed & tested by the community

Retest fail, retest pass

webchick credited joelpittet.

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
commented

If you, like me, were wondering why plain old if (isset($this->options[$option])) would not be a sufficient check here, @joseph.olstad helpfully explains in #2760419-19: get_option() micro optimization that this is because it's possible for $this->options[$option] to deliberately be set to NULL, which would fail the isset() check, but would be caught in the array_key_exists() check. Learn something new every day! :)

Yay for faster code! :D

Committed and pushed to 8.5.x and cherry-picked to 8.4.x.

  • webchick committed 21abcfd on 8.5.x 
    Issue #2903843 by joseph.olstad, joelpittet: views get_option() micro...

  • webchick committed fbfbdbc on 8.4.x 
    Issue #2903843 by joseph.olstad, joelpittet: views get_option() micro...
webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
commented
Status: Reviewed & tested by the community » Fixed
joseph.olstad’s picture

joseph.olstad
Primary language French
commented

Thanks @webchick. props to joel_pittet for the original D7 patch which is essentially identical other than the file, project, line number and branch (LOL).

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.