caliban icon indicating copy to clipboard operation
caliban copied to clipboard

Schema, ArgBuilder and Executor optimizations

Open kyri-petrou opened this issue 1 year ago • 3 comments

⚠️ Binary incompatible changes (for v2.6.0)

Main changes:

  1. Schema and ArgBuilder are now abstract classes instead of traits. The benefit of this change is hard to quantify since it's highly dependent on the number of schemas that exist in an application
  2. ReducedStep is now an abstract class, which improves performance of the isPure method call
  3. Reworked list reduction so that it's done in a single pass

Note on (3): Due to all the optimizations done in the Executor over the past few months, there was too much code which made things harder to read and follow. As an attempt to make it a bit more readable, I separated the logic used for step reduction and reduced step execution into separate classes. Let me know what you think about that - I can revert those changes if we don't like it

kyri-petrou avatar Feb 17 '24 08:02 kyri-petrou

Is it worth potentially breaking user code if there's no clear performance gain? (talking about the first change)

ghostdogpr avatar Feb 18 '24 05:02 ghostdogpr

Is it worth potentially breaking user code if there's no clear performance gain? (talking about the first change)

After thinking about it a bit more, I think that since it's difficult to prove that there will be any quantifiable benefit from changing Schema and ArgBuilder into abstract classes, we should leave them as is for now and perhaps re-consider it in the future if we have proof that it's indeed more performant. I'll revert the changes to those 2

kyri-petrou avatar Feb 19 '24 02:02 kyri-petrou

Let's merge and make the next release 2.6.0

ghostdogpr avatar Feb 29 '24 23:02 ghostdogpr