Skip to content

Fix performance regression in fm_renumber#828

Merged
mboynes merged 2 commits intomainfrom
hotfix/displayif-performance
Mar 1, 2022
Merged

Fix performance regression in fm_renumber#828
mboynes merged 2 commits intomainfrom
hotfix/displayif-performance

Conversation

@mboynes
Copy link
Copy Markdown
Contributor

@mboynes mboynes commented Mar 1, 2022

The performance regression from 1.3 → 1.4 was due to this traversal:

$( '.fm-element[name^="fmtemp_"], .fm-incrementable[name^="fmtemp_"]' ).each( ... )

Since the script knows which elements it changed, this update stores a reference to each changed element to avoid the later DOM traversal.

While this fixes the regression from 1.3 → 1.4, fm_renumber()is still very slow at scale because of all the DOM traversal. A significant refactor is in order.

Resolves #827


For posterity, here's some sample code to produce a very slow experience adding and sorting...

add_action(
	'init',
	function() {
		register_post_type(
			'demo',
			[
				'labels'   => [ 'name' => 'Demos' ],
				'public'   => false,
				'show_ui'  => true,
				'supports' => [ 'title' ],
			]
		);
	}
);

add_action(
	'fm_post_demo',
	function() {
		// Make a big array of sample data.
		$options = range(1, 100);
		$modules = [];
		foreach ( $options as $option ) {
			$modules[ $option ] = new Fieldmanager_Group( get_panel_args( $option ) );
		}

		$fm = new Fieldmanager_Group([
			'name' => 'modules',
			'label' => 'Modules',
			'children' => [
				'module' => new Fieldmanager_Group([
					'label' => 'New Module',
					'label_macro' => [ 'Module: %s', 'module_name' ],
					'limit' => 0,
					'add_more_label' => 'Add another module',
					'collapsible' => true,
					'collapsed' => true,
					'sortable' => true,
					'starting_count' => 1,
					'extra_elements' => 0,
					'children' => array_merge(
						[
							'type' => new Fieldmanager_Select([
								'label' => 'Module Type',
								'options' => $options,
							]),
							'module_name' => new Fieldmanager_TextField([
								'label' => 'Module Name',
							]),
						],
						$modules
					),
				]),
			],
		]);
		$fm->add_meta_box( 'Page Modules', ['demo']);
	}
);

function get_panel_args( $key ) {
	return [
		'label' => $key,
		'display_if' => [
			'src' => 'type',
			'value' => $key,
		],
		'children' => [
			'intro' => new Fieldmanager_RichTextArea('Intro'),
			'items' => new Fieldmanager_Group([
				'label' => 'Item',
				'label_macro' => [ 'Item: %s', 'name' ],
				'limit' => 0,
				'add_more_label' =>  'Add another item',
				'collapsible' => true,
				'collapsed' => true,
				'sortable' => true,
				'starting_count' => 1,
				'extra_elements' => 0,
				'children' => [
					'name' => new Fieldmanager_TextField('Name'),
					'type' => new Fieldmanager_Select([
						'label' => 'Type',
						'type_ahead' => true,
						'first_empty' => true,
						'options' => [
							'nested1' => 'Nested 1',
							'nested2' => 'Nested 2',
						],
					]),
				],
			]),
		],
	];
}

Copy link
Copy Markdown
Contributor

@stevenslack stevenslack left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I pulled down the branch and added the sample code to reproduce the issue, while it is still slow, it is a slight improvement. The largest timeout I received on this branch was 770ms while on the main branch I received some timeouts upwards of 12229ms!!

I don't see anything in the code which is a blocker, but reading the code is a bit nuanced as there are no docblocks or comments in this JavaScript. Comments with some context could help speed up a refactor, but that may be out of scope for this fix.

🌶

@mboynes mboynes merged commit 158cd10 into main Mar 1, 2022
@mboynes mboynes deleted the hotfix/displayif-performance branch March 1, 2022 22:13
@stevenslack
Copy link
Copy Markdown
Contributor

@mboynes The comments are very helpful!

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.

Performance regression in fm_renumber in fieldmanager 1.4.0

2 participants