Skip to content
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

Lightbox: Re-use existing Tag Processor instance. #5428

Draft
wants to merge 1 commit into
base: trunk
Choose a base branch
from

Conversation

dmsnell
Copy link
Member

@dmsnell dmsnell commented Oct 8, 2023

Notes:

  • this file is brought over from the Gutenberg packages. it will need to be updated in Gutenberg and back-ported into this side, but it's being explored here to make it easier to perform some performance analysis.
  • this is showing in preliminary testing an extremely significant but unremarkable improvement, on the order of saving 0.8 ms on a ~95 ms render, on a 2.0 GHz server.

lightbox-single-tag-processor-violins

Description:

Resolves WordPress/gutenberg#55123

Replaces existing single-letter-named Tag Processor instances with a single processor that's reuse throughout the function.

As a work-in-progress (WIP) patch, this is a preliminary exploration and not ready for review or merging.

cc: @artemiomorales @afercia

dmsnell added a commit to WordPress/gutenberg that referenced this pull request Oct 11, 2023
Companion work in WordPress/wordpress-develop#5428

Resolves #55123

This patch refactors the use of the HTML API in image lightbox rendering.
It replaces creating multiple Tag Processor instances with the use of a
single instance that rewinds and jumps around to perform multiple jobs
on a single instance.

Also incorporated is a fix for bailing out when no images are in a given
image block's output HTML.
@dmsnell dmsnell force-pushed the update/lightbox-html-api-use branch 3 times, most recently from f924e03 to fcb9061 Compare October 12, 2023 00:08
Resolves WordPress/gutenberg#55123

Replaces existing single-letter-named Tag Processor instances with
a single processor that's reuse throughout the function.

As a work-in-progress (WIP) patch, this is a preliminary exploration
and not ready for review or merging.
@dmsnell dmsnell force-pushed the update/lightbox-html-api-use branch from fcb9061 to e47a3d0 Compare October 12, 2023 00:09
dmsnell added a commit to WordPress/gutenberg that referenced this pull request Oct 15, 2023
Companion work in WordPress/wordpress-develop#5428

Resolves #55123

This patch refactors the use of the HTML API in image lightbox rendering.
It replaces creating multiple Tag Processor instances with the use of a
single instance that rewinds and jumps around to perform multiple jobs
on a single instance.

Also incorporated is a fix for bailing out when no images are in a given
image block's output HTML.
dmsnell added a commit to WordPress/gutenberg that referenced this pull request Oct 16, 2023
Companion work in WordPress/wordpress-develop#5428

Resolves #55123

This patch refactors the use of the HTML API in image lightbox rendering.
It replaces creating multiple Tag Processor instances with the use of a
single instance that rewinds and jumps around to perform multiple jobs
on a single instance.

Also incorporated is a fix for bailing out when no images are in a given
image block's output HTML.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Image block: clean-up and improve usage of WP_HTML_Tag_Processor
1 participant