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

Optimization: \= on <symbol> is slower than it should be #899

Closed
waywardmonkeys opened this Issue May 9, 2015 · 2 comments

Comments

Projects
None yet
2 participants
@waywardmonkeys
Member

waywardmonkeys commented May 9, 2015

\= on <symbol> does a lot of work!

define sealed inline method \=
    (symbol-1 :: <symbol>, symbol-2 :: <symbol>) => (well? :: <boolean>)
  as(<string>, symbol-1) = as(<string>, symbol-2)
end method \=;

We could store a hash or something like Gwydion did, but at the least, it should check \== first, I think if it isn't going to compare hashes or something.

Right now, this expands into code that even includes some type checks:

  T29 = SLOT_VALUE_INITD(T7, 0);
  T30_0 = T29;
  T31_0 = T30_0;
  {
    MV_CHECK_TYPE_PROLOGUE(T30_0);
    primitive_type_check(T31_0, &KLbyte_stringGVKd);
    MV_CHECK_TYPE_EPILOGUE();
  }
  T32 = T31_0;
  T33 = SLOT_VALUE_INITD(T6, 0);
  T34_0 = T33;
  T35_0 = T34_0;
  {
    MV_CHECK_TYPE_PROLOGUE(T34_0);
    primitive_type_check(T35_0, &KLbyte_stringGVKd);
    MV_CHECK_TYPE_EPILOGUE();
  }
  T36 = T35_0;
  T37_0 = KEVKdMM27I(T32, T36);
  T38 = T37_0;

The type checks are an interesting anomaly. I wasn't expecting that. (I haven't dug enough to see what's up.)

@waywardmonkeys

This comment has been minimized.

Show comment
Hide comment
@waywardmonkeys

waywardmonkeys May 13, 2015

Member

Just did an experimental bootstrap with this change:

-define sealed inline method \=
+define sealed method slow-=
     (symbol-1 :: <symbol>, symbol-2 :: <symbol>) => (well? :: <boolean>)
   as(<string>, symbol-1) = as(<string>, symbol-2)
+end method slow-=;
+
+define sealed inline method \=
+    (symbol-1 :: <symbol>, symbol-2 :: <symbol>) => (well? :: <boolean>)
+  (symbol-1 == symbol-2) | slow-=(symbol-1, symbol-2)
 end method \=;

And these are the locations in a build of dylan-compiler that call \= rather than \==. I know there are more when y ou build dylan-environment:

    // .../sources/dfmc/c-back-end/c-emit-object.dylan:240
    T6_0 = Kslow_EVKiMM0I(T3, T4);
--
    // .../sources/dfmc/definitions/top-level-convert.dylan:41
    T13_0 = Kslow_EVKiMM0I(T11, IKJboot_dylan_definitions_);
--
    // .../sources/environment/dfmc/database/library-objects.dylan:62
    T30_0 = Kslow_EVKiMM0I(T7, T6);
--
    // .../sources/lib/dood/table-proxy.dylan:406
    T4_0 = Kslow_EVKiMM0I(x_, y_);
--
    // .../sources/dylan/symbol.dylan:32
    T3_0 = Kslow_EVKiMM0I(symbol_1_, symbol_2_);
--
    // .../sources/environment/protocols/applications.dylan:166
    T4_0 = Kslow_EVKiMM0I(T2, IKJclosed_);
--
    // .../sources/environment/protocols/applications.dylan:161
    T4_0 = Kslow_EVKiMM0I(T2, IKJstopped_);
--
    // .../sources/environment/protocols/applications.dylan:156
    T4_0 = Kslow_EVKiMM0I(T2, IKJrunning_);
--
    // .../sources/environment/reports/library-report-rst.dylan:211
    T11_0 = Kslow_EVKiMM0I(kindF6, &KJinput_);
--
    // .../sources/lib/jam/jam-reader.dylan:255
    T16_0 = Kslow_EVKiMM0I(T14, IKJpunctuation_);
--
    // .../sources/system/file-system/unix-file-system.dylan:208
    T24_0 = Kslow_EVKiMM0I(if_existsF5, &KJsignal_);
--
    // .../sources/system/file-system/unix-file-system.dylan:181
    T18_0 = Kslow_EVKiMM0I(if_existsF5, &KJsignal_);
--
    // .../sources/lib/walker/walker.dylan:66
    T5_0 = Kslow_EVKiMM0I(T1, &KJvirtual_);
Member

waywardmonkeys commented May 13, 2015

Just did an experimental bootstrap with this change:

-define sealed inline method \=
+define sealed method slow-=
     (symbol-1 :: <symbol>, symbol-2 :: <symbol>) => (well? :: <boolean>)
   as(<string>, symbol-1) = as(<string>, symbol-2)
+end method slow-=;
+
+define sealed inline method \=
+    (symbol-1 :: <symbol>, symbol-2 :: <symbol>) => (well? :: <boolean>)
+  (symbol-1 == symbol-2) | slow-=(symbol-1, symbol-2)
 end method \=;

And these are the locations in a build of dylan-compiler that call \= rather than \==. I know there are more when y ou build dylan-environment:

    // .../sources/dfmc/c-back-end/c-emit-object.dylan:240
    T6_0 = Kslow_EVKiMM0I(T3, T4);
--
    // .../sources/dfmc/definitions/top-level-convert.dylan:41
    T13_0 = Kslow_EVKiMM0I(T11, IKJboot_dylan_definitions_);
--
    // .../sources/environment/dfmc/database/library-objects.dylan:62
    T30_0 = Kslow_EVKiMM0I(T7, T6);
--
    // .../sources/lib/dood/table-proxy.dylan:406
    T4_0 = Kslow_EVKiMM0I(x_, y_);
--
    // .../sources/dylan/symbol.dylan:32
    T3_0 = Kslow_EVKiMM0I(symbol_1_, symbol_2_);
--
    // .../sources/environment/protocols/applications.dylan:166
    T4_0 = Kslow_EVKiMM0I(T2, IKJclosed_);
--
    // .../sources/environment/protocols/applications.dylan:161
    T4_0 = Kslow_EVKiMM0I(T2, IKJstopped_);
--
    // .../sources/environment/protocols/applications.dylan:156
    T4_0 = Kslow_EVKiMM0I(T2, IKJrunning_);
--
    // .../sources/environment/reports/library-report-rst.dylan:211
    T11_0 = Kslow_EVKiMM0I(kindF6, &KJinput_);
--
    // .../sources/lib/jam/jam-reader.dylan:255
    T16_0 = Kslow_EVKiMM0I(T14, IKJpunctuation_);
--
    // .../sources/system/file-system/unix-file-system.dylan:208
    T24_0 = Kslow_EVKiMM0I(if_existsF5, &KJsignal_);
--
    // .../sources/system/file-system/unix-file-system.dylan:181
    T18_0 = Kslow_EVKiMM0I(if_existsF5, &KJsignal_);
--
    // .../sources/lib/walker/walker.dylan:66
    T5_0 = Kslow_EVKiMM0I(T1, &KJvirtual_);
@housel

This comment has been minimized.

Show comment
Hide comment
@housel

housel Jun 1, 2015

Member

Redefining the method as:

define sealed inline method \=
    (symbol-1 :: <symbol>, symbol-2 :: <symbol>) => (well? :: <boolean>)
  let a = as(<string>, symbol-1) = as(<string>, symbol-2);
  let b = symbol-1 == symbol-2;
  assert(a == b);
  a
end method \=;

and doing a three-stage bootstrap succeeds, indicating that the comparing symbol-name method and the pointer equality check method are equivalent, as indeed they should be.

Member

housel commented Jun 1, 2015

Redefining the method as:

define sealed inline method \=
    (symbol-1 :: <symbol>, symbol-2 :: <symbol>) => (well? :: <boolean>)
  let a = as(<string>, symbol-1) = as(<string>, symbol-2);
  let b = symbol-1 == symbol-2;
  assert(a == b);
  a
end method \=;

and doing a three-stage bootstrap succeeds, indicating that the comparing symbol-name method and the pointer equality check method are equivalent, as indeed they should be.

waywardmonkeys added a commit to waywardmonkeys/opendylan that referenced this issue Jun 3, 2015

[dylan] Optimize symbol comparison when using \=.
This makes \= on <symbol> instances be the same as calling \==
instead of going through costly conversions to strings, type
checks and more.

Fixes #899.

* sources/dylan/symbol.dylan
  (\= on <symbol>): Implement using \== rather than a conversion
    to strings and comparing the strings.

waywardmonkeys added a commit to waywardmonkeys/opendylan that referenced this issue Jun 3, 2015

[dylan] Optimize symbol comparison when using \=.
This makes \= on <symbol> instances be the same as calling \==
instead of going through costly conversions to strings, type
checks and more.

Fixes #899.

* sources/dylan/symbol.dylan
  (\= on <symbol>): Implement using \== rather than a conversion
    to strings and comparing the strings.

* documentation/release-notes/source/2015.1.rst: Discuss this change.

@waywardmonkeys waywardmonkeys self-assigned this Jun 3, 2015

@housel housel closed this in #919 Jun 3, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment