From c4fa11e63e5cd3ad84c5b2443019ca9bd801fc71 Mon Sep 17 00:00:00 2001 From: Simon Sapin Date: Thu, 16 Mar 2017 19:13:37 -0700 Subject: [PATCH] servo: Merge #15992 - Rewrite PropertyDeclaration::id to help the optimizer (from servo:id-table); r=bholley MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit If I’m reading the release-mode assembly correctly, before this change `PropertyDeclaration::id` is implemented with a computed jump: ```assembly lea rcx, [rip + .LJTI117_0] movsxd rax, dword ptr [rcx + 4*rax] add rax, rcx jmp rax .LBB117_3: mov dword ptr [rdi], 65536 mov rax, rdi ret .LBB117_2: mov dword ptr [rdi], 0 mov rax, rdi ret .LBB117_4: mov dword ptr [rdi], 131072 mov rax, rdi ret .LBB117_6: mov dword ptr [rdi], 262144 mov rax, rdi ret .LBB117_7: mov dword ptr [rdi], 327680 mov rax, rdi ret ; Four similar lines repeated for each of the few hundred variants... ``` With Rust 1.15 (currently used for geckolib) this doesn’t change significantly. In Nightly 1.17 however, the compiled code uses a lookup table, possibly thanks to https://github.com/rust-lang/rust/pull/39456. ```assembly movq (%rsi), %rax cmpq $171, %rax jne .LBB23_1 addq $8, %rsi movq %rsi, 8(%rdi) movb $1, %al jmp .LBB23_3 .LBB23_1: xorq $128, %rax leaq .Lswitch.table.6(%rip), %rcx movb (%rax,%rcx), %al movb %al, 1(%rdi) xorl %eax, %eax .LBB23_3: movb %al, (%rdi) movq %rdi, %rax retq ``` Source-Repo: https://github.com/servo/servo Source-Revision: 9e8e1a47241c6906b4f5777da0d04e3655982fae --- .../style/properties/properties.mako.rs | 27 ++++++++++++++----- 1 file changed, 21 insertions(+), 6 deletions(-) diff --git a/servo/components/style/properties/properties.mako.rs b/servo/components/style/properties/properties.mako.rs index b0d2b878bfc86..5dca7bee0ccad 100644 --- a/servo/components/style/properties/properties.mako.rs +++ b/servo/components/style/properties/properties.mako.rs @@ -1094,17 +1094,32 @@ impl PropertyDeclaration { /// Given a property declaration, return the property declaration id. pub fn id(&self) -> PropertyDeclarationId { match *self { + PropertyDeclaration::Custom(ref name, _) => { + return PropertyDeclarationId::Custom(name) + } + PropertyDeclaration::CSSWideKeyword(id, _) | + PropertyDeclaration::WithVariables(id, _) => { + return PropertyDeclarationId::Longhand(id) + } + _ => {} + } + let longhand_id = match *self { % for property in data.longhands: PropertyDeclaration::${property.camel_case}(..) => { - PropertyDeclarationId::Longhand(LonghandId::${property.camel_case}) + LonghandId::${property.camel_case} } % endfor - PropertyDeclaration::CSSWideKeyword(id, _) => PropertyDeclarationId::Longhand(id), - PropertyDeclaration::WithVariables(id, _) => PropertyDeclarationId::Longhand(id), - PropertyDeclaration::Custom(ref name, _) => { - PropertyDeclarationId::Custom(name) + PropertyDeclaration::CSSWideKeyword(..) | + PropertyDeclaration::WithVariables(..) | + PropertyDeclaration::Custom(..) => { + debug_assert!(false, "unreachable"); + // This value is never used, but having an expression of the same "shape" + // as for other variants helps the optimizer compile this `match` expression + // to a lookup table. + LonghandId::BackgroundColor } - } + }; + PropertyDeclarationId::Longhand(longhand_id) } fn with_variables_from_shorthand(&self, shorthand: ShorthandId) -> Option< &str> {