General code generation cleanups #3248

mhevery · 2015-07-23T21:58:45Z

Normalize string / No interpolation

Replace interpolate1 = "" "${componentName0 == null ? "" : componentName0}" "" with interpolate1 = " " + stringify(componentName0) + " "; NOTE: will require import of stringify method. I think ${} creates extra unneeded checks, so simple string concat + should be better in this case.

Inline assignments

take advantage of multiple assignments per line.

``` Dart``
_componentName0 = _gen.ChangeDetectionUtil.uninitialized();
_interpolate1 = _gen.ChangeDetectionUtil.uninitialized();
_directive_1_0 = _gen.ChangeDetectionUtil.uninitialized();
_directive_2_0 = _gen.ChangeDetectionUtil.uninitialized();


becomes:

``` Dart
 _componentName0 = _interpolate1 = _directive_1_0 = _directive_2_0 
       = _gen.ChangeDetectionUtil.uninitialized;

Also call dehydrateDirectives() from constructor so that we don't have to duplicate the code in the constructor.

Inline assignments and usages

inline where ever possible.

        _dispatcher.notifyOnBinding(currentProto.bindingRecord, interpolate1);
        _interpolate1 = interpolate1;

becomes

        _dispatcher.notifyOnBinding(currentProto.bindingRecord, _interpolate1 = interpolate1);

have `notifyOnBinding` helper

create viewSet in super. Notice that _dispatcher is always a View. This code is no longer abstract so we should use concrete names.

currentProto = _protos[1]
...     
change_interpolate1 = true;
_dispatcher.notifyOnBinding(currentProto.bindingRecord, interpolate1);
_interpolate1 = interpolate1;

becomes

        change_interpolate1 = viewSet(1, _interpolate1 = interpolate1);

which requires this method on superclass

  boolean viewSet(int index, dynamic value) {
    _view.notifyOnBinding(protos[index].bindingRecord, value);
    return true;
  }

`looseNotIdentical`

have loseNotIdentical which will allow us to change if (!_gen.looseIdentical(...)) to if (_gen.looseNotIdentical(...)). (its only one char, but it is everywhere)

inline context assignment

    var context = null;
    context = _context;

should be

    var context = _context;

Unused `change_context`

the code is never used in output

var change_context = false;

but is never read.

change `uninitialized()` method call to `uninitialized` static field

should save some chars

The text was updated successfully, but these errors were encountered:

kegluneq · 2015-07-23T22:44:35Z

fyi, per our discussion i'm currently working on Inline assignments above

kegluneq · 2015-07-24T21:26:21Z

Step 1: Determine if "Normalize string / No interpolation" provides any benefit
Step 2 already underway

Create `NameRegistry`, responsible for understanding how names are generated for change detector fields and variables. Use `NameRegistry` for both JS Jit & Dart pre-generated detectors. Making progress on angular#3248

Create `NameRegistry`, responsible for understanding how names are generated for change detector fields and variables. Use `NameRegistry` for both JS Jit & Dart pre-generated detectors. Making progress on #3248

Previously, `uninitialized()` was a method, requiring a call as well as two extra characters everywhere it was used. Make this value a variable, saving the characters and avoiding the method call to get its value. This change also removes the export of `uninitialized` from change_detect.ts, which is technically a breaking change, however `uninitialized` is an implementation detail and nobody should be using it in app logic. By convention, apps should not be importing from files under `src/`. Update to angular#3248.

Create `looseNotIdentical => !looseIdentical`, which will save a lot of unnecessary '!' characters in generated change detectors. Update to angular#3248

Add additional details here! Update to angular#3248

Call new `(de)hydrateDirectives` methods from `(de)hydrate`. Add a null implementation in `AbstractChangeDetector` and only override if necessary for the specific change detector. Update to angular#3248

Create `looseNotIdentical => !looseIdentical`, which will save a lot of unnecessary '!' characters in generated change detectors. Update to angular#3248

yjbanov · 2015-07-28T19:45:14Z

This issue proposes to optimize the size of code that we should not be generating at all. Therefore, fixing any of these will have 0 net effect on code size once we stop generating it. I'd rather we spent our time on the ultimate solution.

The ultimate solution is to move all value fields, and comparison and notification logic into common classes. Generated code should only be responsible for evaluating expressions in a typed and monomorphic manner. Here are two ways (which we can combine) for how we can do this: https://docs.google.com/document/d/1x8MmbK3bKJb5JFrz3SmLfUsWYURZhYqPux12zKm1i8k/edit#

kegluneq · 2015-07-28T20:50:22Z

@yjbanov , I commented on that doc but in summary I'm not confident we'll be able to implement that scheme without performance impact. Most of the ProtoRecords result in code like what you propose but not all of them and we'd need runtime checks to figure out what course of action to take.

In the mean time, this process is cutting down on code duplication, which should make any ultimate solution easier to implement.

Call new `(de)hydrateDirectives` methods from `(de)hydrate`. Add a null implementation in `AbstractChangeDetector` and only override if necessary for the specific change detector. Update to angular#3248

Move fields common to Dynamic, Jit, and Pregen change detectors into the `AbstractChangeDetector` superclass to save on codegen size and reduce code duplication. Update to angular#3248

Move fields common to Dynamic, Jit, and Pregen change detectors into the `AbstractChangeDetector` superclass to save on codegen size and reduce code duplication. Update to angular#3248, closes angular#3243

kegluneq · 2015-07-29T18:09:31Z

Double-checked, but at least for Dart, interpolation is better than + concat, so it's my intention to not complete that piece of this issue unless someone wants to discuss.
I'm pushing the remaining parts to the next milestone because Refactor hydrate/dehydrate/hydrated to superclass #3245 will be a bigger win and is closer to completion
- Inline assignments and usages
- have notifyOnBinding helper

I won't be finishing either this milestone as I'm out the next couple of days.

kegluneq · 2015-08-11T22:41:42Z

The remaining points here are not as urgent as supporting parts in the transformer.

kegluneq · 2016-02-02T17:54:52Z

Change detectors & code generation in general have changed a lot - this is now obsolete.

angular-automatic-lock-bot · 2019-09-07T22:22:03Z

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

_{This action has been performed automatically by a bot.}

mhevery added comp: dart-transformer effort2: days refactoring Issue that involves refactoring or code-cleanup labels Jul 23, 2015

mhevery changed the title ~~Dart code generation cleanups~~ General code generation cleanups Jul 23, 2015

kegluneq assigned kevmoo and kegluneq and unassigned kevmoo Jul 24, 2015

kegluneq mentioned this issue Jul 27, 2015

refactor(change_detect): Make ChangeDetectionUtil#uninitialized a var #3320

Merged

kegluneq mentioned this issue Jul 28, 2015

refactor(change_detect): Create & use looseNotIdentical #3322

Merged

kegluneq pushed a commit to kegluneq/angular that referenced this issue Jul 28, 2015

refactor(change_detect): Move (de)hydrate logic into dedicated methods

2dcda86

Add additional details here! Update to angular#3248

kegluneq pushed a commit to kegluneq/angular that referenced this issue Jul 28, 2015

refactor(change_detect): Move (de)hydrate logic into dedicated methods

18c4a25

Add additional details here! Update to angular#3248

kegluneq mentioned this issue Jul 28, 2015

refactor(change_detect): Move (de)hydrate logic into dedicated methods #3332

Merged

kegluneq added this to the alpha-34 milestone Jul 28, 2015

yjbanov added P2: required area: performance and removed P1: urgent refactoring Issue that involves refactoring or code-cleanup labels Jul 28, 2015

kegluneq mentioned this issue Jul 28, 2015

refactor(change_detect): Move common fields to AbstractChangeDetector #3347

Merged

kegluneq modified the milestones: alpha-35, alpha-34 Jul 29, 2015

mhevery added P3: important and removed P2: required labels Aug 4, 2015

kegluneq modified the milestones: alpha-36, alpha-35 Aug 11, 2015

mhevery added state: Periodic Review and removed state: Periodic Review labels Aug 20, 2015

mhevery modified the milestones: alpha-36, alpha-37 Sep 1, 2015

naomiblack modified the milestones: alpha-37, alpha-39 Oct 4, 2015

IgorMinar modified the milestone: alpha-39 Oct 5, 2015

kegluneq closed this as completed Feb 2, 2016

angular-automatic-lock-bot bot locked and limited conversation to collaborators Sep 7, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

General code generation cleanups #3248

General code generation cleanups #3248

mhevery commented Jul 23, 2015

kegluneq commented Jul 23, 2015

kegluneq commented Jul 24, 2015

yjbanov commented Jul 28, 2015

kegluneq commented Jul 28, 2015

kegluneq commented Jul 29, 2015

kegluneq commented Aug 11, 2015

kegluneq commented Feb 2, 2016

angular-automatic-lock-bot bot commented Sep 7, 2019

General code generation cleanups #3248

General code generation cleanups #3248

Comments

mhevery commented Jul 23, 2015

Normalize string / No interpolation

Inline assignments

Inline assignments and usages

have notifyOnBinding helper

looseNotIdentical

inline context assignment

Unused change_context

change uninitialized() method call to uninitialized static field

kegluneq commented Jul 23, 2015

kegluneq commented Jul 24, 2015

yjbanov commented Jul 28, 2015

kegluneq commented Jul 28, 2015

kegluneq commented Jul 29, 2015

kegluneq commented Aug 11, 2015

kegluneq commented Feb 2, 2016

angular-automatic-lock-bot bot commented Sep 7, 2019

have `notifyOnBinding` helper

`looseNotIdentical`

Unused `change_context`

change `uninitialized()` method call to `uninitialized` static field