Skip to content

Optimizations lost in uncurried mode #6879

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

Closed
3 tasks done
cknitt opened this issue Jul 14, 2024 · 6 comments
Closed
3 tasks done

Optimizations lost in uncurried mode #6879

cknitt opened this issue Jul 14, 2024 · 6 comments
Milestone

Comments

@cknitt
Copy link
Member

cknitt commented Jul 14, 2024

  • Inlining lost in jscomp/gentype_tests/typescript-react-example/src/Records.res
  • Temporary variable introduced in jscomp/test/bs_poly_mutable_set_test.res
  • Belt_SetString.fromArray/toArray example

This is a follow-up to my comments on the diffs in #6864.

Inlining lost in the output for jscomp/gentype_tests/typescript-react-example/src/Records.res:

let getOpt = (opt, default, foo) => opt->Option.mapWithDefault(default, foo)

Before (curried mode):

let getOpt = Belt_Option.mapWithDefault;

After (uncurried mode):

function getOpt(opt, $$default, foo) {
  return Belt_Option.mapWithDefault(opt, $$default, foo);
}

Temporary variable introduced in jscomp/test/bs_poly_mutable_set_test.res:

Input:

b(__LOC__, N.isEmpty(N.intersect(aa, bb)))

Before (curried mode):

b("File \"bs_poly_mutable_set_test.res\", line 96, characters 4-11", Belt_MutableSet.isEmpty(Belt_MutableSet.intersect(aa, bb)));

After (uncurried mode):

let d$1 = Belt_MutableSet.intersect(aa, bb);

b("File \"bs_poly_mutable_set_test.res\", line 96, characters 4-11", d$1.data === undefined);

(OTOH isEmpty is now inlined here which is good.)

@cknitt
Copy link
Member Author

cknitt commented Jul 17, 2024

Related example: When Belt is compiled in uncurried mode, the following code

open RescriptSchema

let stringSet = S.array(S.string)->S.transform(_ => {
  parser: a => Belt.Set.String.fromArray(a),
  serializer: a => Belt.Set.String.toArray(a),
})

compiles to

let stringSet = S$RescriptSchema.transform(S$RescriptSchema.array(S$RescriptSchema.string), (function (param) {
  return {
    p: (function (a) {
      return Belt_SetString.fromArray(a);
    }),
    s: (function (a) {
      return Belt_SetString.toArray(a);
    })
  };
}));

whereas when Belt is compiled in curried mode, it compiles to

let stringSet = S$RescriptSchema.transform(S$RescriptSchema.array(S$RescriptSchema.string), (function (param) {
  return {
    p: Belt_SetString.fromArray,
    s: Belt_SetString.toArray
  };
}));

@cristianoc
Copy link
Collaborator

foo(1,2) is transformed to Js.Internal.opaqueFullApply(Internal.opaque(f), 1, 2).
This is used to prevent transformations that apply by default and assume curried semantics.
These in turn prevent certain optimisations.

It's probably worth investigating what transformations can be gradually removed so opaqueFullApplyand opaque are not needed anymore, and see what the consequences are.

@cristianoc cristianoc added this to the v12 milestone Jul 19, 2024
@cristianoc
Copy link
Collaborator

It seems that opaque is not actually needed (at least at the moment after recent changes): #6892

@cristianoc
Copy link
Collaborator

Instead, here's the effect of removing opaqueFullApply: https://github.com/rescript-lang/rescript-compiler/pull/6893/files
A bunch of applications are considered curried applications.

@cristianoc
Copy link
Collaborator

cristianoc commented Jul 19, 2024

This then blindly changes every application at the lambda level to use App_uncurry: https://github.com/rescript-lang/rescript-compiler/pull/6894/files
A lot of inlining is unlocked that way. Bunch of wrong code too, but the direction seems worth exploring.

@cknitt
Copy link
Member Author

cknitt commented Jul 25, 2024

Everything resolved in current master.

@cknitt cknitt closed this as completed Jul 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants