Skip to content

script: Replace raw pointer comparison with == for DOM identity comparison - #46718

Merged
kkoyung merged 1 commit into
servo:mainfrom
yezhizhen:equality
Jul 22, 2026
Merged

script: Replace raw pointer comparison with == for DOM identity comparison#46718
kkoyung merged 1 commit into
servo:mainfrom
yezhizhen:equality

Conversation

@yezhizhen

@yezhizhen yezhizhen commented Jul 22, 2026

Copy link
Copy Markdown
Member

As mentioned in #46678 (comment), we should rely on PartialEq for all cases, so that we just need to optimize a single point.

Testing: This would be slightly slower because of

impl PartialEq for DomElement {
fn eq(&self, other: &Self) -> bool {
crate::DomObject::reflector(self) == crate::DomObject::reflector(other)
, which then expands into multiple crates and call chain. But other behaviours should not change.

Part of #34464

.
Signed-off-by: Euclid Ye <yezhizhenjiakang@gmail.com>
@yezhizhen
yezhizhen requested review from sagudev and xiaochengh July 22, 2026 03:16
@yezhizhen
yezhizhen requested a review from gterzian as a code owner July 22, 2026 03:16
@servo-highfive servo-highfive added the S-awaiting-review There is new code that needs to be reviewed. label Jul 22, 2026
@servo-highfive servo-highfive removed the S-awaiting-review There is new code that needs to be reviewed. label Jul 22, 2026
@kkoyung
kkoyung added this pull request to the merge queue Jul 22, 2026
@servo-highfive servo-highfive added the S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. label Jul 22, 2026
Merged via the queue into servo:main with commit 28d6327 Jul 22, 2026
35 checks passed
@servo-highfive servo-highfive removed the S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. label Jul 22, 2026
@yezhizhen
yezhizhen deleted the equality branch July 22, 2026 06:05
pull Bot pushed a commit to xtqqczze/servo-servo that referenced this pull request Jul 27, 2026
Unlike claimed in
servo#46678 (comment) and
servo#46718, the result from benchmark is
counter-intuitive.
`std::ptr::eq` is actually 15% slower than
https://github.com/servo/servo/blob/fc340087b7411d5d736ad8705581baf16b567b1c/components/dom_struct/lib.rs#L154-L156.

The benchmark mimics the one used in
servo#35323.

Testing: 
`.\mach exec cargo bench -p servo-script-bindings`
```
Running benches.rs (target\release\deps\dom_partial_eq-8ef5af22310fd8f7.exe)
ptr_eq_same             time:   [598.35 ps 602.22 ps 606.18 ps]
Found 4 outliers among 100 measurements (4.00%)
  1 (1.00%) low mild
  1 (1.00%) high mild
  2 (2.00%) high severe

ptr_eq_different        time:   [615.17 ps 619.21 ps 623.09 ps]
Found 3 outliers among 100 measurements (3.00%)
  1 (1.00%) high mild
  2 (2.00%) high severe

dom_eq_same             time:   [514.00 ps 519.76 ps 525.78 ps]
Found 5 outliers among 100 measurements (5.00%)
  3 (3.00%) high mild
  2 (2.00%) high severe

dom_eq_different        time:   [514.45 ps 520.22 ps 526.39 ps]
Found 6 outliers among 100 measurements (6.00%)
  1 (1.00%) low mild
  4 (4.00%) high mild
  1 (1.00%) high severe
```

Also, production profile `.\mach exec cargo bench --profile production
-p servo-script-bindings` is always slower than default.

---------

Signed-off-by: Euclid Ye <yezhizhenjiakang@gmail.com>
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.

3 participants