Announcement

Collapse
No announcement yet.

[CLOSED] [#921] Lift simple anonymous functions into named-functions for performance wins

Collapse
X
  • Filter
  • Time
  • Show
Clear All
new posts

  • [CLOSED] [#921] Lift simple anonymous functions into named-functions for performance wins

    Bear with me on this one, it's going to take a bit of explaining!

    I have an idea that comes partly from some work I've been doing with React and from articles I've been reading about optimising React UIs for cases where there are huge numbers of elements. It also comes from articles relating to more extreme performance requirements in JavaScript (ie. talk of "60 FPS on mobile") and it comes from some talks by Colt McAnlis that I've been listening to ("The Joys of Static Memory JavaScript" is making me "scared of closures" - this guy gives good talk!).

    The short version is that nested functions in JavaScript have a cost as they are created on-the-fly every single time that the parent function is executed. To take a really simple example from one of the articles I've been looking at (http://code.tutsplus.com/tutorials/s...hem--net-22315):

    function foo(a, b) {
        function bar() {
            return a + b;
        }
     
        return bar();
    }
    The "bar" function is created every time that "foo" is executed. So if "foo" is executed a million times, "foo" itself is only created once, but the "bar" function is created a million times. This puts additional strain on the garbage collector. If the above code had been written as:

    function foo(a, b) {
        return bar(a, b);
    }
     
    function bar(a, b) {
        return a + b;
    }
    then both "foo" and "bar" are each created only once.

    (Apologies, by the way, if I'm teaching you how to suck eggs*, I just wanted to start at the beginning because I've only just gotten straight in my head precisely what it is that I think that I want).

    * (Is this a British phrase?? Further apologies if you have no idea what I'm talking about :D)

    That's not the main point that I want to make, though - but it is related! Hopefully I'll be able to explain how.. My starting point with this request is with React. React has this fantastic facility called the "Virtual DOM" which means that you don't have to write code that can initialise your UI and then write code that can deal with applying changes to it - what you do in React is only have the code to initialise the UI for a given state. Then, for every change, you totally re-render this Virtual DOM. This makes writing the code simple - it's basically a pure function; given this state, render these DOM elements. The clever bit being that, although the real browser DOM is very slow, the Virtual DOM is very fast and it takes the "entire page re-render" requests and then just works out which little bits have changed and then only applies these changes to the real DOM. So your code can be simple and the DOM rendering can be fast.

    Much of the time, this is great and React's Virtual DOM is fast enough to take whatever you throw at it. However, if you've got a really complicated UI (or you have more extreme performance requirements), React does allow you to give it some hints to make it even faster. Each Component can (if it chooses to) implement a function "ShouldComponentUpdate". This function returns a boolean. If it returns true, then the Virtual DOM does its usual work - the Virtual DOM re-renders and then any changes required are applied to the browser DOM. However, if it returns false then the Virtual DOM does not re-render the component, nor does it re-render any of its child components. So, in a very complicated page, this could mean that React can perform less work in its Virtual DOM mechanism for each update, if the person writing the code is able to come up with some useful "ShouldComponentUpdate" implementations. This is quite easy if immutable structures are used for all components' "State" and "Props" references, since referential equality comparisons may be used to see if any of the data has changed - if the "ShouldComponentUpdate" implementation can be sure that the current State and Props references contain the same data as the new State and Props references, then there is no reason to bother re-rendering the component.

    This is great in theory, but it falls apart if any of the properties on a "Props" reference is an anonymous function (lambda). To try to illustrate what I mean, I've got an example React component -

    public class AppContainerComponent : Component<object, AppContainerComponent.State>
    {
        public AppContainerComponent() : base(null) { }
    
        public override ReactElement Render()
        {
            return DOM.Div(new Attributes { ClassName = "my-app" },
                new TitleComponent(
                    text: state.Title,
                    onChange: newTitle => SetState(state.With(_ => _.Title, newTitle))
                ),
                new MessageEditListComponent(
                    messages: state.Messages,
                    onChange: (id, newMessageState) => SetState(state.With(_ => _.Messages, UpdateMessage(state.Messages, id, newMessageState)))
                )
            );
        }
    
        protected override State GetInitialState()
        {
            return new State(
                "My Demo App!",
                Set.Of(
                    Persisted.New(1, new MessageEditState("Hi!", "Tim")),
                    Persisted.New(2, new MessageEditState("Bye", "Dave"))
                )
            );
        }
    
        private Set<Persisted<MessageEditState>> UpdateMessage(Set<Persisted<MessageEditState>> messages, int id, MessageEditState newMessageState)
        {
            var messageToUpdate = messages.FirstOrDefault(message => message.Id == id);
            if ((messageToUpdate == null) || (messageToUpdate.Value == newMessageState))
                return messages;
    
            return messages
                .Select(message => (message.Id == id) ? message.With(_ => _.Value, newMessageState) : message)
                .ToSet();
        }
    
        public class State : IAmImmutable
        {
            public State(string title, Set<Persisted<MessageEditState>> messages)
            {
                this.CtorSet(_ => _.Title, title);
                this.CtorSet(_ => _.Messages, messages);
            }
            public string Title { get; private set; }
            public Set<Persisted<MessageEditState>> Messages { get; private set; }
        }
    }
    This uses my Bridge.React bindings and the ProductiveRage.Immutable library (both available on NuGet).

    The important part is in the "Render" method, where "TitleComponent" and "MessageEditListComponent" components are created. When they are created, they are given information about the current state of the app and they are given callbacks to execute when any user interaction with those components requires a change to the app state. The "TitleComponent" renders a text input box - when the user tries to change the value in the input, the "onChange" callback is executed with the new title value and the whole app re-renders. Likewise, if any change to the currently-displayed state is required following a user interaction with the "MessageEditListComponent" then its "onChange" callback is executed and the app is re-rendered.

    These "onChange" callbacks are defined as lambdas because C#'s type inference makes them so pleasant to write. The "TitleComponent" onChange has an argument "newTitle" of type "string", but I don't have to write out "string" because the compiler knows what it is.

    The problem with this is that if I try to create a "ShouldComponentUpdate" function for these components, it won't be possible. If we imagine that the user has pressed a key and changed the TitleComponent's text input and the app is re-rendered, then it would be ideal if the MessageEditListComponent could look at its new Props when it is re-rendered and say "all of these messages are the same as the messages I'm already showing, and this onChange callback is the same onChange callback that I was given last time; I don't need to re-render anything". But that's not possible because, while all of the messages will be the same, the onChange callback is a function, since it is created dynamically each time that the AppContainerComponent's Render method is executed (just like the "Bar" function is created over and over again in my first example).

    One way around this would be to avoid using lambdas in component callback properties. The "AppContainerComponent" class could be changed to the following:

    public class AppContainerComponent : Component<object, AppContainerComponent.State>
    {
        public AppContainerComponent() : base(null) { }
    
        public override ReactElement Render()
        {
            return DOM.Div(new Attributes { ClassName = "my-app" },
                new TitleComponent(
                    text: state.Title,
                    onChange: OnTitleChange
                ),
                new MessageEditListComponent(
                    messages: state.Messages,
                    onChange: OnMessageChange
                )
            );
        }
    
        private void OnTitleChange(string newTitle)
        {
            SetState(state.With(_ => _.Title, newTitle));
        }
    
        private void OnMessageChange(int id, MessageEditState newMessageState)
        {
            SetState(state.With(_ => _.Messages, UpdateMessage(state.Messages, id, newMessageState)));
        }
    
        protected override State GetInitialState()
        {
            return new State(
                "My Demo App!",
                Set.Of(
                    Persisted.New(1, new MessageEditState("Hi!", "Tim")),
                    Persisted.New(2, new MessageEditState("Bye", "Dave"))
                )
            );
        }
    
        private Set<Persisted<MessageEditState>> UpdateMessage(Set<Persisted<MessageEditState>> messages, int id, MessageEditState newMessageState)
        {
            var messageToUpdate = messages.FirstOrDefault(message => message.Id == id);
            if ((messageToUpdate == null) || (messageToUpdate.Value == newMessageState))
                return messages;
    
            return messages
                .Select(message => (message.Id == id) ? message.With(_ => _.Value, newMessageState) : message)
                .ToSet();
        }
    
        public class State : IAmImmutable
        {
            public State(string title, Set<Persisted<MessageEditState>> messages)
            {
                this.CtorSet(_ => _.Title, title);
                this.CtorSet(_ => _.Messages, messages);
            }
            public string Title { get; private set; }
            public Set<Persisted<MessageEditState>> Messages { get; private set; }
        }
    }
    The downside to this is that I don't get to rely on C#'s type inference so much and I have to write out the types for "newTitle" in the "OnTitleChange" method and "id" and "newMessageState" in the "OnMessageChange" method. This isn't a huge deal, but for some components these methods signatures can get more complicated and having to write out the types explicitly adds more noise to the code.

    The other downside is that this is something that a developer needs to be vigilant about. There will not be any compiler warning if they don't use these named functions, but the child components will not be able to include effective "ShouldComponentUpdate" implementations since the callbacks will always be new each time.

    What I'm proposing is a new option in the bridge.json configuration that will convert "trivial lambdas" into named class functions as part of the Bridge translation process. This would mean that lambdas could be used in the C# code, but they would not become dynamically-created functions in the JavaScript. This would slightly reduce GC load in cases where there were no ShouldComponentUpdate implementations, as fewer objects would be created during React / Virtual DOM re-renders. However, it could significantly reduce Virtual DOM work (and subsequently significantly reduce GC load as fewer Render calls would be required) in apps where ShouldComponentUpdate implementations are present.

    I would define a "trivial lambda" as one whose method body only accesses its arguments and / or class-scoped members (functions, properties or fields). Hopefully, this would make the pre-translation work fairly straight-forward (by "pre-translation work", I mean that I'm imagining a translator pass that changes C# with trivial lambdas into C# with those trivial lambdas changed into named functions - and that this would happen before the C#-to-JavaScript translation occurs). The first "foo" / "bar" example is actually quite a lot more complicated that a "trivial lambda" as the "bar" method requires a closure that allows access to the variables "a" and "b" that are local to "foo" - these variables had to be passed to "bar" as arguments in the re-worked example. If a lambda only accesses its own arguments and / or class-scoped members, then its method signature does not need to change.

    Even if only "trivial lambdas" are supported, I still don't imagine that this is a particularly trivial change. And I would not be surprised or upset if this was not something that would ever makes its way into v1, since so much focus is on getting v2 ready to replace it. But I thought that I would put it out there because I think there could be a lot of value to this change. Particularly because I'm working on a new component base class for the React bindings - the "PureComponent". This may be used by stateless components whose "Props" types are known to be immutable. The base class will automatically implement "ShouldComponentUpdate", allowing for React to work as efficiently as possible with the minimum of effort for the developer. Very simple in principle to React's "PureRenderMixin" (though mixins are not supported in the Bridge.React bindings, nor are they supported for people writing React components in ES6 - which seems to be the direction that Facebook are steering people towards these days). It will not be possible for the "PureComponent" to be as effective as I would like, though, if anonymous function callbacks are provided as properties on component Props instances in the generated JavaScript.

    I do have some related thoughts and questions on my own suggestion.. firstly, if this option was enabled then it might affect debugging a little bit, because there would be C# code that had an inline anonymous function, that then becomes a named JavaScript function. That might be a little bit jarring. (But if v2 is supporting source maps then you could do something clever to map the inside of the named function body back on to the anonymous function body in the C# code). Secondly, is any sort of "pre-translation work" like this supported in either Bridge v1 or v2 via plugins? I'm not completely sure what the plugin architecture is for, or how it is used. If it is possible with a plugin, would I distribute that plugin with my Bridge.React bindings if the PureComponent base class is relying on that trivial-lambda-to-named-function transformation being applied? And will IPlugin exist in its current incarnation in v2? It appears to be tied to NRefactory, so I presume would change to use Roslyn if IPlugin lives on in v2?

    Finally, I've implied in the above essay that it's easy to compare two instances of a named function to see if they are the same. This isn't strictly true, since named functions used as properties on a Props instance would have to be bound to the instance of the caller. Bridge does this automatically using Bridge.fn.bind - when it does this, it returns a function object with properties "$scope" and "$method". When comparing two functions, I'm considering them to be equivalent if they are either the same reference (obviously) or if they both have "$scope" and "$method" properties and that both "$scope" values are the same reference and both "$method" values are the same reference. I'm quite aware that I'm relying on Bridge internals here, is this behaviour likely to change in the future (ie. in v2)? Is there a safer mechanism that I could use?

    Just to really explode this post into (most likely) the longest forum post ever, below is a complete example that illustrates how "PureComponent" would be written (including the $scope / $method sniffing). It also has some lambdas that are commented as being trivial (ie. elligible for lifting into named class-scoped functions) and one that couldn't (that would unfortunately need to be left as an anonymous function - see the "MessageEditListComponent" class).

    using System;
    using System.Linq;
    using Bridge;
    using Bridge.Html5;
    using Bridge.React;
    using ProductiveRage.Immutable;
    
    namespace Example
    {
        class Program
        {
            [Ready]
            static void Main()
            {
                React.Render(
                    new AppContainerComponent(),
                    Document.GetElementById("main")
                );
            }
        }
    
        /// <summary>
        /// In this example, this Component does not require any Props data and so a TProps type parameter of "object" is used
        /// </summary>
        public class AppContainerComponent : Component<object, AppContainerComponent.State>
        {
            public AppContainerComponent() : base(null) { }
    
            public override ReactElement Render()
            {
                // Both "onChange" lambdas here would be elligible for being extracted into named functions since they only access the lambda
                // arguments ("newTitle", "id", "newMessageState") and class-instance-scoped functions ("SetState" and "UpdateMessage") and
                // class-instance-scoped variables ("state")
                return DOM.Div(new Attributes { ClassName = "my-app" },
                    new TitleComponent(
                        text: state.Title,
                        onChange: newTitle => SetState(state.With(_ => _.Title, newTitle))
                    ),
                    new MessageEditListComponent(
                        messages: state.Messages,
                        onChange: (id, newMessageState) => SetState(state.With(_ => _.Messages, UpdateMessage(state.Messages, id, newMessageState)))
                    )
                );
            }
    
            protected override State GetInitialState()
            {
                return new State(
                    "My Demo App!",
                    Set.Of(
                        Persisted.New(1, new MessageEditState("Hi!", "Tim")),
                        Persisted.New(2, new MessageEditState("Bye", "Dave"))
                    )
                );
            }
    
            private Set<Persisted<MessageEditState>> UpdateMessage(Set<Persisted<MessageEditState>> messages, int id, MessageEditState newMessageState)
            {
                var messageToUpdate = messages.FirstOrDefault(message => message.Id == id);
                if ((messageToUpdate == null) || (messageToUpdate.Value == newMessageState))
                    return messages;
    
            return messages
                .Select(message => (message.Id == id) ? message.With(_ => _.Value, newMessageState) : message)
                .ToSet();
            }
    
            public class State : IAmImmutable
            {
                public State(string title, Set<Persisted<MessageEditState>> messages)
                {
                    this.CtorSet(_ => _.Title, title);
                    this.CtorSet(_ => _.Messages, messages);
                }
                public string Title { get; private set; }
                public Set<Persisted<MessageEditState>> Messages { get; private set; }
            }
        }
    
        public class TitleComponent : PureComponent<TitleComponent.Props>
        {
            public TitleComponent(string text, Action<string> onChange) : base(new Props(text, onChange)) { }
    
            public override ReactElement Render()
            {
                return DOM.Div(new Attributes { ClassName = "title" },
                    DOM.Label(null, "Title: "),
                    DOM.Input(new InputAttributes
                    {
                        // The OnChange lambda here would be a candidate for extracting into a named function as it only accesses the lambda
                        // argument ("e") and a class-instance-scoped reference ("props")
                        Value = props.Text,
                        OnChange = e => props.OnChange(e.CurrentTarget.Value)
                    })
                );
            }
    
            public class Props : IAmImmutable
            {
                public Props(string text, Action<string> onChange)
                {
                    this.CtorSet(_ => _.Text, text);
                    this.CtorSet(_ => _.OnChange, onChange);
                }
                public string Text { get; private set; }
                public Action<string> OnChange { get; private set; }
            }
        }
    
        public class MessageEditListComponent : PureComponent<MessageEditListComponent.Props>
        {
            public MessageEditListComponent(Set<Persisted<MessageEditState>> messages, Action<int, MessageEditState> onChange) : base(new Props(messages, onChange)) { }
    
            public override ReactElement Render()
            {
                var messageRows = props.Messages
                    .Select(message => new MessageEditRowComponent(
                        // Note: The OnChange lambda here could NOT be pulled out into a named function in the proposed scheme as it accesses
                        // the local variable "message". The accessing of the class-instance-scoped "props" reference is ok, as is the lambda
                        // argument "newMessageState", but supporting local variables like this would make things much more complicated.
                        key: message.Id,
                        message: message.Value,
                        onChange: newMessageState => props.OnChange(message.Id, newMessageState)
                    ))
                    .Select(component => (Any<ReactElement, string>)component);
                return DOM.FieldSet(new FieldSetAttributes { ClassName = "message-edit-list" },
                    DOM.Legend(null, "Messages"),
                    DOM.Div(null, messageRows.ToArray())
                );
            }
    
            public class Props : IAmImmutable
            {
                public Props(Set<Persisted<MessageEditState>> messages, Action<int, MessageEditState> onChange)
                {
                    this.CtorSet(_ => _.Messages, messages);
                    this.CtorSet(_ => _.OnChange, onChange);
                }
                public Set<Persisted<MessageEditState>> Messages { get; private set; }
                public Action<int, MessageEditState> OnChange { get; private set; }
            }
        }
    
        public class MessageEditRowComponent : PureComponent<MessageEditRowComponent.Props>
        {
            public MessageEditRowComponent(int key, MessageEditState message, Action<MessageEditState> onChange) : base(new Props(key, message, onChange)) { }
    
            public override ReactElement Render()
            {
                return DOM.Div(new Attributes { ClassName = "message-edit-row" },
                    DOM.Input(new InputAttributes
                    {
                        // The OnChange lambda here would be a candidate for extracting into a named function as it only accesses the lambda
                        // argument ("e") and a class-instance-scoped reference ("props")
                        Value = props.Message.Content,
                        OnChange = e => props.OnChange(props.Message.With(_ => _.Content, e.CurrentTarget.Value))
                    }),
                    DOM.Label(null, " (Author: " + props.Message.Author + ")")
                );
            }
    
            public class Props : IAmImmutable
            {
                public Props(int key, MessageEditState message, Action<MessageEditState> onChange)
                {
                    this.CtorSet(_ => _.Key, key);
                    this.CtorSet(_ => _.Message, message);
                    this.CtorSet(_ => _.OnChange, onChange);
                }
                public int Key { get; private set; }
                public MessageEditState Message { get; private set; }
                public Action<MessageEditState> OnChange { get; private set; }
            }
        }
    
        /// <summary>
        /// A Persisted instance is a value with an id, the implication being that this record has been maintained in a data store somewhere and that if
        /// any edits to it are saved then they will be updating an existing record somewhere (a value may exist that is not persisted, meaning that a
        /// new record would be written if it was saved). A Message Editing component may not know whether the current value is persisted or not since
        /// that fact does not affect its implementation - one of the parent component would need to know, though.
        /// </summary>
        public class Persisted<T> : IAmImmutable
        {
            public Persisted(int id, T value)
            {
                this.CtorSet(_ => _.Id, id);
                this.CtorSet(_ => _.Value, value);
            }
            public int Id { get; private set; }
            public T Value { get; private set; }
        }
        public static class Persisted
        {
            /// <summary>
            /// This is just to take advantage of C#'s type inference when initialising a Persisted instance
            /// </summary>
            public static Persisted<T> New<T>(int id, T value)
            {
                return new Persisted<T>(id, value);
            }
        }
    
    
        public class MessageEditState : IAmImmutable
        {
            public MessageEditState(string content, string author)
            {
                this.CtorSet(_ => _.Content, content);
                this.CtorSet(_ => _.Author, author);
            }
            public string Content { get; private set; }
            public string Author { get; private set; }
        }
    
        /// <summary>
        /// A Pure Component should only vary its output based upon the provided Props reference. The data type used for the Props reference must
        /// be fully-immutable, meaning that is all of the properties on two Props instances are the same references then the two instances contain
        /// the same data - this means that no properties or nested properties may be mutable. Since it will vary only by Props data, State is not
        /// not supported.
        /// </summary>
        public abstract class PureComponent<TProps> : Component<TProps, object>
        {
            protected PureComponent(TProps props, params Any<ReactElement, string>[] children) : base(props, children) { }
    
            protected override bool ShouldComponentUpdate(TProps nextProps, object nextState)
            {
                // If the current props and the props are different then we need to re-render the component - otherwise, we don't!
                if (!DoPropsReferencesMatch(props, nextProps))
                    return true;
    
                Console.WriteLine(this.GetClassName().Split(".").Last() + " didn't have to re-render - hurrah!");
                return false;
            }
    
            private static bool DoPropsReferencesMatch(TProps props1, TProps props2)
            {
                if ((props1 == null) && (props2 == null))
                    return true;
                else if ((props1 == null) || (props2 == null))
                    return false;
    
                if (props1.GetType() != props2.GetType())
                    return false;
    
                // Note: This relies upon some Bridge.NET internals (namely that the "Bridge.fn.bind" function always sets $scope and
                // $method properties on the generated function)
                /*@
                for (var propName in props1) {
                    var propValue1 = props1[propName];
                    var propValue2 = props2[propName];
                    if ((propValue1 === propValue2)
                    || ((propValue1 === null) && (propValue2 === null))
                    || ((typeof(propValue1) === "undefined") && (typeof(propValue2) === "undefined"))) {
                        continue;
                    }
                    else if ((typeof(propValue1) === "function") && (typeof(propValue2) === "function")) {
                        if (propValue1.$scope && propValue1.$method && propValue2.$scope && propValue2.$method && (propValue1.$scope === propValue2.$scope) && (propValue1.$method === propValue2.$method)) {
                            continue;
                        }
                    }
                    return false;
                }
                */
                return true;
            }
        }
    
        public static class IEnumerableExtensions
        {
            public static Set<T> ToSet<T>(this IEnumerable<T> source)
            {
                return Set.Of(source.ToArray());
            }
        }
    }
    As always, thanks for your time dealing with my requests - I look forward to hearing your thoughts!
    Last edited by Daniil; 2016-02-07 @ 08:36 PM.

  • #2
    Epic. We're going to need a little time to digest this.

    Comment


    • #3
      Hi
      We commited initial implementation of that feature
      You can test it from Issue921 branch (https://github.com/bridgedotnet/Bridge/tree/Issue921)
      We will appreciate any feedback

      Comment


      • #4
        It is in the branch link, but still - a direct link to the associated GitHub issue:
        https://github.com/bridgedotnet/Bridge/issues/921

        Comment


        • #5
          Just want to show changes

          C# code
          namespace Demo
          {
              public class App
              {
                  [Ready]
                  public static void Main()
                  {
                      string[] names = { "Daniil", "Fabricio", "Geoffrey", "Leonid", "Ozgur" };
                      IEnumerable query = names.Where(n => n.Contains("i")).OrderBy(n => n.Length);
                  }
              }
          }
          Old output (without converting anonymous functions to named)
          (function (globals) {
              "use strict";
          
              Bridge.define('Demo.App', {
                  statics: {
                      config: {
                          init: function () {
                              Bridge.ready(this.main);
                          }
                      },
                      main: function () {
                          var names = ["Daniil", "Fabricio", "Geoffrey", "Leonid", "Ozgur"];
                          var query = Bridge.Linq.Enumerable.from(names).where(function (n) {
                              return Bridge.String.contains(n,"i");
                          }).orderBy(function (n) {
                              return n.length;
                          });
                      }
                  }
              });
              
              Bridge.init();
          })(this);
          New output
          (function (globals) {
              "use strict";
          
              Bridge.define('Demo.App', {
                  statics: {
                      config: {
                          init: function () {
                              Bridge.ready(this.main);
                          }
                      },
                      main: function () {
                          var names = ["Daniil", "Fabricio", "Geoffrey", "Leonid", "Ozgur"];
                          var query = Bridge.Linq.Enumerable.from(names).where(Demo.App.$f1).orderBy(Demo.App.$f2);
                      },
                      $f1: function (n) {
                          return Bridge.String.contains(n,"i");
                      },
                      $f2: function (n) {
                          return n.length;
                      }
                  }
              });
              
              Bridge.init();
          })(this);

          Comment


          • #6
            I've glanced through the changeset in github and looked at the example here and it looks exactly like what I wanted! Hoping to be able to try it out properly tomorrow, I'll be back in touch with either feedback or just thanks! :)

            Comment


            • #7
              I've had a little play around with this in a test project, just to try it out.. and I'm really impressed - this is great work!

              This is the code that exercises the obvious test cases that I could think of:

              using System.Collections.Generic;
              using System.Linq;
              using Bridge.Html5;
              
              namespace BridgeLambdaLiftPreview
              {
                  public class Class1
                  {
                      [Ready]
                      public static void Go()
                      {
                          Console.WriteLine(string.Join(", ", (new Example(123)).GetValues()));
                      }
                  }
              
                  public class Example
                  {
                      private readonly int _offset;
                      public Example(int offset)
                      {
                          _offset = offset;
                      }
              
                      public IEnumerable<int> GetValues()
                      {
                          var localValue = 456;
                          return
                              new[] { 1, 2, 3 }
                              .Select(value => value + 1) // Simple lift (single-variable)
                              .Select(value => value + 1) // Simple lift (single-variable) - repeat
                              .Select((value, index) => value + index) // Simple lift (two-variable)
                              .Select(value => value + _offset) // Lift and bind (single-variable)
                              .Select((value, index) => value + index + _offset) // Lift and bind (two-variable)
                              .Select(value => value + localValue); // Non-lift
                      }
                  }
              }
              And they all work perfectly!

              When "this" is referenced (or a variable that is scoped to the parent "this") then the Bridge.fn.bind is used and otherwise it isn't. When local variables are used then the lambda is not lifted into a named function. This seems absolutely ideal.

              /* global Bridge */
              
              (function (globals) {
                  "use strict";
              
                  Bridge.define('BridgeLambdaLiftPreview.Example', {
                      _offset: 0,
                      constructor: function (offset) {
                          this._offset = offset;
                      },
                      getValues: function () {
                          var localValue = 456;
                          return Bridge.Linq.Enumerable.from([1, 2, 3])
                              .select(BridgeLambdaLiftPreview.Example.$f1)
                              .select(BridgeLambdaLiftPreview.Example.$f2)
                              .select(BridgeLambdaLiftPreview.Example.$f3)
                              .select(Bridge.fn.bind(this, BridgeLambdaLiftPreview.Example.$f4))
                              .select(Bridge.fn.bind(this, BridgeLambdaLiftPreview.Example.$f5))
                              .select(function (value) {
                                  return value + localValue;
                              }); // Non-lift
                      },
                      statics: {
                          $f1: function (value) {
                              return value + 1;
                          },
                          $f2: function (value) {
                              return value + 1;
                          },
                          $f3: function (value, index) {
                              return value + index;
                          },
                          $f4: function (value) {
                              return value + this._offset;
                          },
                          $f5: function (value, index) {
                              return value + index + this._offset;
                          }
                      }
                  });
                  
                  Bridge.define('BridgeLambdaLiftPreview.Class1', {
                      statics: {
                          config: {
                              init: function () {
                                  Bridge.ready(this.go);
                              }
                          },
                          go: function () {
                              console.log(Bridge.toArray((new BridgeLambdaLiftPreview.Example(123)).getValues()).join(", "));
                          }
                      }
                  });
                  
                  Bridge.init();
              })(this);
              Very, very minor points to mention. Firstly, after pasting in the JavaScript above, I've noticed that the comments that followed the lambdas in the C# have been lost in the lift-to-named-functions cases (eg. "// Simple lift (single-variable)"). Secondly, the functions $f1 and $f2 have the exact same contents and both exist within the same scope - it could make the generated code slightly smaller to remove duplicates like this.

              Overall, though, this is fantastic! Features like this are able to make Bridge better not just because I can write my client-side code in C# but also because it can make it easy for me to produce well-performing JavaScript. I'm hoping to finish a blog post this week about some performance enhancements for the React bindings and I will certainly be talking about this change since it will directly impact the changes to the bindings that I'm making. (If this code isn't ready for release yet, that's not a problem - I will describe it as something that will hopefully be landing soon and, when it does, will automatically make the React performance boosts work even better).

              Comment


              • #8
                As a final confirmation, I've taken the Bridge "Issue-921 Build" and tested it against more complicated code (a variation on the React example included above), rather than my simple test cases above, and it worked perfectly. Thanks again!

                Comment


                • #9
                  Dan, thank you very much for testing and the feedback. So happy to hear it works very well.

                  Very, very minor points to mention. Firstly, after pasting in the JavaScript above, I've noticed that the comments that followed the lambdas in the C# have been lost in the lift-to-named-functions cases (eg. "// Simple lift (single-variable)"). Secondly, the functions $f1 and $f2 have the exact same contents and both exist within the same scope - it could make the generated code slightly smaller to remove duplicates like this.
                  Thanks for these. We'll investigate.

                  Comment


                  • #10
                    I've got another sort-of related feature request, but it's sufficiently different that I've created another thread: http://forums.bridge.net/forum/gener...tion-signature

                    Comment


                    • #11
                      Firstly, after pasting in the JavaScript above, I've noticed that the comments that followed the lambdas in the C# have been lost in the lift-to-named-functions cases (eg. "// Simple lift (single-variable)").
                      I have reproduced, although no fix has been implemented. I haven't decided where these comments would be rendered either. Should they be moved inside the lifted function definitions, or above the lifted function definitions, or should the comments remain associated to the caller somehow? If remain with the caller, because this is a chained sequence of function calls, it's still unclear where the comment would be placed in that scenario. More thinking required.

                      At the moment this is low priority, and might have to wait till 1.12. Unless we figure something easy to do with these comments.

                      Secondly, the functions $f1 and $f2 have the exact same contents and both exist within the same scope - it could make the generated code slightly smaller to remove duplicates like this.
                      Fixed in Branch #921.

                      Comment


                      • #12
                        Fair point about the comments. I would probably expect them to be pulled into the lifted function since the comment content seems more likely to refer to lamba body, but there are no guarantees, it could equally apply to the chaining process.

                        Do you mean that the comment fix (whatever it may be) would be a 1.12 feature but that the lambda lifting would be 1.11?

                        Comment


                        • #13
                          Do you mean that the comment fix (whatever it may be) would be a 1.12 feature
                          Well, there is still a chance it will go to 1.11, but if not it will be slated to 1.12. At least, if there is no showstopper for technical reasons (NRefactory-related or such). Created an individual issue:
                          https://github.com/bridgedotnet/Bridge/issues/961

                          We just discussed the issue. Probably, the following makes the best sense? To emit comments where it is actually in C#, like:

                          // C#
                          .Select(value => value + 1) // Simple lift (single-variable)
                          
                          // JavaScript
                          .select(BridgeLambdaLiftPreview.Example.$f1) // Simple lift (single-variable)

                          Comment


                          • #14
                            but that the lambda lifting would be 1.11?
                            Yes, it is going to be merged to 1.11.

                            Comment


                            • #15
                              The issue is that we do not render the chained functions on separate lines, we concat into one line.

                              So adding a trailing comment would not work, unless we change the chained function format (separate lines). This functionality is already likely to change in v1.12, as we move away from chained functions and into nested functions. Related to Issue #712.

                              https://github.com/bridgedotnet/Bridge/issues/712

                              Comment

                              Working...
                              X