Skip to content
This repository has been archived by the owner on May 2, 2024. It is now read-only.

Mention that the generated code is not comparable to hand-written one. #2

Closed
cristianoc opened this issue Nov 21, 2019 · 1 comment
Closed

Comments

@cristianoc
Copy link

Some mention should go in the README that the generated code is not comparable to the handwritten one, in terms of perf. So one should be aware of that.

The "alternative without the syntax sugar" in the README has a few typos. I believe it should be this:

let getStreet = (maybeUser: option(user)): option(string) => {
  maybeUser->Belt.Option.flatMap(user =>
    user.info
    ->Belt.Option.flatMap(personalInfo =>
        personalInfo.address
        ->Belt.Option.flatMap(address =>
            address.street
            ->Belt.Option.flatMap(street =>
                Some(street->Js.String.toUpperCase)
              )
          )
      )
  );
};

The JS output:

function getStreet(maybeUser) {
  return Belt_Option.flatMap(maybeUser, (function (user) {
                return Belt_Option.flatMap(user[/* info */0], (function (personalInfo) {
                              return Belt_Option.flatMap(personalInfo[/* address */0], (function (address) {
                                            return Belt_Option.flatMap(address[/* street */0], (function (street) {
                                                          return street.toUpperCase();
                                                        }));
                                          }));
                            }));
              }));
}

So that's more or less (or perhaps exactly) the code produced by the ppx.

Consider instead this other handwritten version:

let getStreetExplicit = (maybeUser: option(user)): option(string) => {
  switch (maybeUser) {
  | None => None
  | Some(user) =>
    switch (user.info) {
    | None => None
    | Some(personalInfo) =>
      switch (personalInfo.address) {
      | None => None
      | Some(address) =>
        switch (address.street) {
        | None => None
        | Some(street) => Some(street->Js.String.toUpperCase)
        }
      }
    }
  };
};

and the generated JS:

function getStreetExplicit(maybeUser) {
  if (maybeUser !== undefined) {
    var match = maybeUser[/* info */ 0];
    if (match !== undefined) {
      var match$1 = match[/* address */ 0];
      if (match$1 !== undefined) {
        var match$2 = match$1[/* street */ 0];
        if (match$2 !== undefined) {
          return match$2.toUpperCase();
        } else {
          return;
        }
      } else {
        return;
      }
    } else {
      return;
    }
  }
}

There is a very substantial difference in the perf and allocation behavior of this generated code.

Note: with enough compiler optimizations in future, it might be possible to generate this directly from the code written using the PPX.

@mrmurphy
Copy link
Contributor

awesome. Thanks for the detailed issue. I'll update the ReadMe

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants