Consider using instanceof checks for DOM Nodes instead of DOMUtils::is* + DOMUtils::assertElt combinations
Closed, ResolvedPublic
Actions

Assigned To

Authored By

	ssastry
	May 14 2019, 4:20 AM

Description

instanceof checks to test for text / element / comment node is much cheaper than using DOMUtils::is* helpers along with a DOMUtils::assertElt helper to satisfy phan.

Details

	Subject	Repo	Branch	Lines +/-
	Replace some instances of DOMUtils::assertElt	mediawiki/services/parsoid	master	+49 -45
	Replace DOMUtils::isElt && assertElt(..) combo with instanceof checks	mediawiki/services/parsoid	master	+29 -54

Customize query in gerrit

Related Objects

Duplicates Merged Here: T229740: Replace DOMUtils::assertElt($node) with ( $node instanceof DOMElement ) wherever feasible

Event Timeline

ssastry created this task.May 14 2019, 4:20 AM

Restricted Application added a subscriber: Aklapper. · View Herald TranscriptMay 14 2019, 4:20 AM

ssastry triaged this task as Medium priority.May 14 2019, 4:20 AM

ssastry moved this task from Backlog to Performance on the Parsoid-PHP board.

ssastry merged a task: T229740: Replace DOMUtils::assertElt($node) with ( $node instanceof DOMElement ) wherever feasible.Aug 4 2019, 3:30 AM

Change 533258 had a related patch set uploaded (by Subramanya Sastry; owner: Subramanya Sastry):
[mediawiki/services/parsoid@master] Replace DOMUtils::isElt && assertElt(..) combo with instanceof checks

https://gerrit.wikimedia.org/r/533258

gerritbot added a project: Patch-For-Review.Aug 29 2019, 4:20 PM

Change 533258 merged by jenkins-bot:
[mediawiki/services/parsoid@master] Replace DOMUtils::isElt && assertElt(..) combo with instanceof checks

https://gerrit.wikimedia.org/r/533258

Maintenance_bot removed a project: Patch-For-Review.Aug 29 2019, 6:10 PM

ssastry moved this task from Performance to Post-Port Work on the Parsoid-PHP board.Sep 10 2019, 4:44 AM

ssastry moved this task from Post-Port Work to Porting Tech Debt Redressal on the Parsoid-PHP board.Sep 11 2019, 6:07 PM

After reviewing Parsoid/PHP on Oct, 9, it appears that all performance improvements possible were addressed by:
https://gerrit.wikimedia.org/r/533258

Change 606258 had a related patch set uploaded (by Arlolra; owner: Arlolra):
[mediawiki/services/parsoid@master] Replace some instances of DOMUtils::assertElt

https://gerrit.wikimedia.org/r/606258

gerritbot added a project: Patch-For-Review.Jun 17 2020, 8:32 PM

Change 606258 merged by jenkins-bot:
[mediawiki/services/parsoid@master] Replace some instances of DOMUtils::assertElt