Skip to content
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

Better fast transformers #465

Closed
sebmck opened this issue Jan 12, 2015 · 26 comments
Closed

Better fast transformers #465

sebmck opened this issue Jan 12, 2015 · 26 comments
Assignees
Labels
outdated A closed issue/PR that is archived due to age. Recommended to make a new issue
Milestone

Comments

@sebmck
Copy link
Contributor

sebmck commented Jan 12, 2015

I'm currently thinking of good strategies to proceed with having fast transformers. Basically, to avoid having a ton of optional transformers I think it'd be best to have a fast flag. Basically you'd do something like 6to5 --fast forOf or transform("code", { fast: ["forOf"] });.

Since adding the classesFastSuper transformer I realised that the current way of having optional transformers isn't really scalable. I'd really like to do a bunch of other stuff such as using assignment vs define on computed properties, classes etc. This would obviously be extensively documented in a "fast" section on the 6to5 docs. To enable all the fast transformers you could just go 6to5 --fast all or something similar.

This would obviously be a breaking change since I'd be removing the current fast optional transformers so it'll either wait until 3.0.0 or be added in a backwards compatible way.

/cc @monsanto

@sebmck sebmck added this to the 3.0.0 milestone Jan 12, 2015
@monsanto
Copy link
Contributor

  • +1 on --fast, grouping them with the optional transforms seemed weird to me
  • not sure about --fast all unless there was a warning in the CLI help, wouldn't want people to just say "well of course I want it to go fast" and enable it without reading the docs
  • another fast transformer that comes to mind would be tagged template literals without the two Object.freeze calls. A tag function modifying an object that by spec is not allowed to be modified is really unusual, and I'm seeing ~120x overhead on Chrome and ~85x overhead on Firefox.

@AluisioASG
Copy link
Contributor

@monsanto Maybe another flag than --fast to convey we're trading spec compliancy for speed?

@zloirock
Copy link
Member

@monsanto totally agree.

@jamiebuilds
Copy link
Contributor

not sure about --fast all unless there was a warning in the CLI help

👍

I would wait until we actually get the request for that and decide what to do, there's only two right now so it's not a big deal to specify each of them.

@sebmck
Copy link
Contributor Author

sebmck commented Jan 12, 2015

@thejameskyle Ideally I'd like to make a ton of the transformers have fast options as there are really nitpicky spec stuff.

@jamiebuilds
Copy link
Contributor

@sebmck Fair enough. It should definitely warn the user wherever possible.

@sebmck
Copy link
Contributor Author

sebmck commented Jan 13, 2015

@thejameskyle bought up a good point on gitter (re: calling them "fast")

I feel like the term might be ambiguous for being transformed faster rather than transforming into code that runs faster

Doesn't anyone have any alternate terminology suggestions? I'm currently sitting on some commits that adds the fast functionality for transformers and it's backwards compatible so we wont have to wait for a major version bump. Really eager to get this sorted out so we can start working on making 6to5 extremely fast for those 99% use-cases. I've considered the quite scary "uncompliant" but it's quite iffy.

@sebmck sebmck removed this from the 3.0.0 milestone Jan 13, 2015
@chrisdwheatley
Copy link

  • --experimental
  • --unstable
  • --non-spec

Just some suggestions but none really stand out above the others, they're all a bit woolly.

@sebmck
Copy link
Contributor Author

sebmck commented Jan 13, 2015

@swirlycheetah --experimental is already currently used to support ES7 proposals that haven't been solidified yet. The others are, yeah, a bit wooly as they aren't entirely reflective of what the "fast" transformers actually do.

@stefanpenner
Copy link
Member

--quick-compile or --fast-compile should prevent the ambiguity between what is actually intended to get faster.

I realize it impractical, but it would be great to understand which spec cases I am opting out of. As I, others and potential future bug reports are likely nervous to have not quite compliant output, although I suspect if the trade offs are documented and well known. The concern may go away entirely.

@sebmck
Copy link
Contributor Author

sebmck commented Jan 13, 2015

@stefanpenner --quick-compile and --fast-compile still imply that the performance is in the compile step instead of the executable code that 6to5 generates.

I absolutely agree on documenting the spec deviations. Whatever documentation is present is going to have very visible warnings as well as extensive documentation on the pros and cons, possible side effects and caveats.

@stefanpenner
Copy link
Member

Maybe following the -o flags from gcc? Although I suspect the target audience may not not all be familiar with those

@sebmck
Copy link
Contributor Author

sebmck commented Jan 13, 2015

Yeah, doesn't seem entirely apt either since they aren't really optimisations.

@sebmck
Copy link
Contributor Author

sebmck commented Jan 13, 2015

How about loose? I think it summarises what we're trying to achieve extremely well.

sebmck added a commit that referenced this issue Jan 13, 2015
@stefanpenner
Copy link
Member

not a serious recommendation, but i had a small personal lol: --fast-and-loose

@sebmck
Copy link
Contributor Author

sebmck commented Jan 13, 2015

@stefanpenner Haha, that was actually a suggestion given to me that lead to just loose.

@sebmck
Copy link
Contributor Author

sebmck commented Jan 13, 2015

Going to use this post as my scratchpad for loose changes, feel free to make suggestions other additions. These will of course be thoroughly documented.

  • Classes
    • super is static.
      • References the static super class so prototype changes subsequentlly aren't reflected.
    • Methods use assigns instead of define.
      • Collisions on prototype.
  • Computed property names
    • Uses assigns instead of defines.
      • Collisions on prototype.
  • Template literals
    • Tagged literals aren't frozen
  • For of
    • Special case for arrays
      • Verbose code

@eventualbuddha
Copy link
Member

We should give examples that fail for each of these in the docs.

@gaearon
Copy link
Member

gaearon commented Jan 13, 2015

+1 for loose.

@monsanto
Copy link
Contributor

loose option for template literals here #480 .

@sebmck
Copy link
Contributor Author

sebmck commented Jan 13, 2015

Released in 2.12.0

@stefanpenner
Copy link
Member

@sebmck your a machine.

@chicoxyzzy
Copy link
Member

There should be a way to apply loose mode to all transformers, for example

{
  loose: true
}

@sebmck
Copy link
Contributor Author

sebmck commented Jan 15, 2015

@chicoxyzzy Wouldn't be possible to set it via the CLI etc then, it'd have to be a string like "all" or a variation.

@chicoxyzzy
Copy link
Member

loose: 'all' seems good too

@sebmck
Copy link
Contributor Author

sebmck commented Jan 18, 2015

I've added loose mode docs. They're a bit iffy and the wording is odd, pull requests improving it would be extremely appreciated. Thanks everyone!

@jamiebuilds jamiebuilds added this to the 3.0.0 milestone Jan 21, 2015
@lock lock bot added the outdated A closed issue/PR that is archived due to age. Recommended to make a new issue label Jan 18, 2019
@lock lock bot locked as resolved and limited conversation to collaborators Jan 18, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
outdated A closed issue/PR that is archived due to age. Recommended to make a new issue
Projects
None yet
Development

No branches or pull requests

10 participants