Minimize features array size

[Legioth]

Order index mapping of feature types according to how often they are
actually used (based on actual use frequencies in Beverage Buddy and
Bakery). This allows allocating a smaller features array for each state
node to only hold the features that are actually instantiated.

The case with only one used feature is further optimized to directly
store the feature instance instead of using a one-item array.

Reduces BasicElementView memory use from 149909 to 136045 bytes and the
memory use in BeverageBuddy with an edit dialog open from 151733 to
146061 bytes.

---

This change is 

[gilberto-torrezan]

Reviewed 2 of 3 files at r1, 1 of 1 files at r2.
Reviewable status: 4 unresolved discussions, 0 of 1 LGTMs obtained

---

flow-server/src/main/java/com/vaadin/flow/internal/StateNode.java, line 360 at r2 (raw file):

    public <T extends NodeFeature> T getFeature(Class<T> featureType) {
        int featureIndex = getFeatureIndex(featureType);

Please add a comment in the code for the future generation of maintainers to understand what is this about (and why a simple array is not used like in previous reviews). I know the comment in PR explains it already, but it is more straightforward to have it on the code as well.

---

flow-server/src/main/java/com/vaadin/flow/internal/StateNode.java, line 371 at r2 (raw file):

                features = new NodeFeature[featureIndex + 1];
            } else if (features instanceof NodeFeature) {
                features = new NodeFeature[] { (NodeFeature) features };

Here you could create the array already with the correct size (to avoid creating it and discarding it a few lines below).

---

flow-server/src/main/java/com/vaadin/flow/internal/nodefeature/NodeFeatureRegistry.java, line 120 at r2 (raw file):

/* Only used for the root node */

Could put this comment over her as well, to be consistent with the test.

---

flow-server/src/test/java/com/vaadin/flow/internal/nodefeature/NodeFeatureTest.java, line 165 at r2 (raw file):

                ReconnectDialogConfigurationMap.class);

        Assert.assertEquals(priorityOrder.size(), expectedOrder.size());

The order here is reversed. It should be assertEquals(expectedOrder.size(), priorityOrder.size()).

[Legioth]

Reviewable status: 4 unresolved discussions, 0 of 1 LGTMs obtained

---

flow-server/src/main/java/com/vaadin/flow/internal/StateNode.java, line 360 at r2 (raw file):

Previously, gilberto-torrezan (Gilberto Torrezan) wrote…

Please add a comment in the code for the future generation of maintainers to understand what is this about (and why a simple array is not used like in previous reviews). I know the comment in PR explains it already, but it is more straightforward to have it on the code as well.

Done.

---

flow-server/src/main/java/com/vaadin/flow/internal/StateNode.java, line 371 at r2 (raw file):

Previously, gilberto-torrezan (Gilberto Torrezan) wrote…

Here you could create the array already with the correct size (to avoid creating it and discarding it a few lines below).

Done.

---

flow-server/src/main/java/com/vaadin/flow/internal/nodefeature/NodeFeatureRegistry.java, line 120 at r2 (raw file):

Previously, gilberto-torrezan (Gilberto Torrezan) wrote…

/* Only used for the root node */

Could put this comment over her as well, to be consistent with the test.

Done.

---

flow-server/src/test/java/com/vaadin/flow/internal/nodefeature/NodeFeatureTest.java, line 165 at r2 (raw file):

Previously, gilberto-torrezan (Gilberto Torrezan) wrote…

The order here is reversed. It should be assertEquals(expectedOrder.size(), priorityOrder.size()).

Done.

[vaadin-bot]

SonarQube analysis reported 1 issue

• 1 major

Watch the comments in this conversation to review them.

[vaadin-bot]

Refactor this method to reduce its Cognitive Complexity from 16 to the 15 allowed.

[gilberto-torrezan]

Reviewed 3 of 3 files at r3.
Reviewable status: 1 unresolved discussion, 0 of 1 LGTMs obtained, and 1 stale

[ZheSun88]

Dismissed @vaadin-bot from a discussion.
Reviewable status: complete! all discussions resolved, 1 of 1 LGTMs obtained