Skip to content

perf: replace LinkedList and Optionals with ArrayDeque and EnumMap#166

Merged
DanySK merged 5 commits intoDanySK:masterfrom
Photon-GitHub:master
Mar 23, 2022
Merged

perf: replace LinkedList and Optionals with ArrayDeque and EnumMap#166
DanySK merged 5 commits intoDanySK:masterfrom
Photon-GitHub:master

Conversation

@Photon-GitHub
Copy link
Copy Markdown
Contributor

@Photon-GitHub Photon-GitHub commented Mar 22, 2022

This pull request mainly does three things:

  1. It swaps out the LinkedList implementation with an ArrayDeque. In most cases (i.e. adding and querying) this will improve performance as we only write or read an array. Moreover this allows to directly init with the default capacity which makes sure the array does not need to be grown. This is faster than the LinkedList which wraps every new entry.
    Only the remove and move calls might be slower, but even in the worst case, we only have 2 arraycopy calls on a size of default_capacity, which should be negligient. Also, I think that these operations are less often used compared to inserting elements and querying, but that is dependent on the use case.

  2. Replace the Optional list of children with an EnumMap. There were a lot of list.get(c.ordinal()).get() calls in the code, which goes against the reason Optional is used (as the isPresent check is missing). Furthermore, the list init is quite hard to understand right now, because the Child enum is not linked to list size (4 is a magic value appearing there) and we always use c.ordinal() as the list index. For this, an EnumMap is much clearer. First of all, it directly reflects the Enum, makes the connection clear and does not need the verbose init of the current list. Removing the optional part was easy, and due to the already often missing isPresent checks I only needed to add one single null (in setChild()) in the entire codebase, and even that would be easy to remove.

  3. Thanks to the removal of the children absent optionals some methods have become much smaller and could be removed, like hasChildren() which is now equal to !children.isEmpty() or getChild(...) which is now children.get()

@Photon-GitHub Photon-GitHub changed the title ArrayDeque and EnumMap refactor: ArrayDeque and EnumMap Mar 22, 2022
@DanySK DanySK changed the title refactor: ArrayDeque and EnumMap perf: replace LinkedList and Optionals with ArrayDeque and EnumMap Mar 23, 2022
@codecov
Copy link
Copy Markdown

codecov Bot commented Mar 23, 2022

Codecov Report

Merging #166 (15054cf) into master (5007123) will decrease coverage by 0.91%.
The diff coverage is 76.47%.

@@             Coverage Diff              @@
##             master     #166      +/-   ##
============================================
- Coverage     76.03%   75.11%   -0.92%     
+ Complexity       79       73       -6     
============================================
  Files             1        1              
  Lines           217      209       -8     
  Branches         38       37       -1     
============================================
- Hits            165      157       -8     
  Misses           25       25              
  Partials         27       27              
Impacted Files Coverage Δ
.../java/org/danilopianini/util/FlexibleQuadTree.java 75.11% <76.47%> (-0.92%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5007123...15054cf. Read the comment docs.

@DanySK
Copy link
Copy Markdown
Owner

DanySK commented Mar 23, 2022

Thank you for contributing.

Looks good. I'm going to squash this in as soon as the tests pass. Since the main motivation for change seems to be performance, I modified the PR type. Also, this should cause the generation of a new release.

@DanySK DanySK merged commit b265ca6 into DanySK:master Mar 23, 2022
github-actions Bot pushed a commit that referenced this pull request Mar 23, 2022
### [0.1.8](0.1.7...0.1.8) (2022-03-23)

### General maintenance

* ignore node_modules ([9ceae26](9ceae26))
* **release:** correctly enable semantic commit conventions ([9dab9e9](9dab9e9))

### Dependency updates

* **deps:** update dependency @semantic-release/exec to v6.0.3 ([a653fdb](a653fdb))
* **deps:** update dependency com.github.spotbugs:spotbugs-annotations to v4.5.2 ([96ba851](96ba851))
* **deps:** update dependency com.github.spotbugs:spotbugs-annotations to v4.5.3 ([08f0d67](08f0d67))
* **deps:** update dependency com.github.spotbugs:spotbugs-annotations to v4.6.0 ([2a65473](2a65473))
* **deps:** update dependency com.google.guava:guava to v31.1-jre ([33443b5](33443b5))
* **deps:** update dependency gradle to v7.3.1 ([61f0a7f](61f0a7f))
* **deps:** update dependency gradle to v7.3.2 ([72a0b75](72a0b75))
* **deps:** update dependency gradle to v7.3.3 ([47b4423](47b4423))
* **deps:** update dependency gradle to v7.4 ([f68a953](f68a953))
* **deps:** update dependency gradle to v7.4.1 ([825784a](825784a))
* **deps:** update dependency node-fetch to 2.6.7 [security] ([3df04e2](3df04e2))
* **deps:** update dependency semantic-release-preconfigured-conventional-commits to v1.1.1 ([fc5687d](fc5687d))
* **deps:** update dependency semantic-release-preconfigured-conventional-commits to v1.1.3 ([4108720](4108720))
* **deps:** update dependency semantic-release-preconfigured-conventional-commits to v1.1.4 ([0970f94](0970f94))
* **deps:** update dependency semantic-release-preconfigured-conventional-commits to v1.1.5 ([2a1ca60](2a1ca60))
* **deps:** update node.js to 16.14 ([09443d4](09443d4))
* **deps:** update plugin java-qa to v0.13.0 ([abd821e](abd821e))
* **deps:** update plugin java-qa to v0.14.0 ([e2695c8](e2695c8))
* **deps:** update plugin java-qa to v0.15.0 ([2062366](2062366))
* **deps:** update plugin java-qa to v0.16.0 ([b04f435](b04f435))
* **deps:** update plugin java-qa to v0.18.0 ([7bd18c8](7bd18c8))
* **deps:** update plugin java-qa to v0.19.0 ([a8058b0](a8058b0))
* **deps:** update plugin java-qa to v0.20.0 ([7a7acda](7a7acda))
* **deps:** update plugin java-qa to v0.20.1 ([38f3a68](38f3a68))
* **deps:** update plugin java-qa to v0.21.0 ([620da9e](620da9e))
* **deps:** update plugin multijvmtesting to v0.3.1 ([a98ee84](a98ee84))
* **deps:** update plugin multijvmtesting to v0.3.2 ([fe7c6a7](fe7c6a7))
* **deps:** update plugin multijvmtesting to v0.3.3 ([79c1326](79c1326))
* **deps:** update plugin multijvmtesting to v0.3.4 ([dfef60d](dfef60d))
* **deps:** update plugin publishoncentral to v0.7.10 ([2391b28](2391b28))
* **deps:** update plugin publishoncentral to v0.7.11 ([a2b2995](a2b2995))
* **deps:** update plugin publishoncentral to v0.7.12 ([ac98fc8](ac98fc8))
* **deps:** update plugin publishoncentral to v0.7.13 ([f48a33d](f48a33d))
* **deps:** update plugin publishoncentral to v0.7.14 ([f73764b](f73764b))
* **deps:** update plugin publishoncentral to v0.7.7 ([c9c488e](c9c488e))
* **deps:** update plugin publishoncentral to v0.7.8 ([2c0d724](2c0d724))
* **deps:** update plugin publishoncentral to v0.7.9 ([1434d6a](1434d6a))

### Build and continuous integration

* **deps:** update actions/checkout action to v3 ([01324d4](01324d4))
* **deps:** update danysk/action-checkout action to v0.2.1 ([acd3a84](acd3a84))
* **deps:** update danysk/build-check-deploy-gradle-action action to v1.1.2 ([ee9075d](ee9075d))
* **deps:** update danysk/build-check-deploy-gradle-action action to v1.1.3 ([157335c](157335c))
* **deps:** update danysk/build-check-deploy-gradle-action action to v1.2.0 ([b781c22](b781c22))
* **deps:** update danysk/build-check-deploy-gradle-action action to v1.2.4 ([59bdee6](59bdee6))
* **deps:** update danysk/build-check-deploy-gradle-action action to v1.2.6 ([863effe](863effe))
* **deps:** update danysk/build-check-deploy-gradle-action action to v1.2.7 ([5007123](5007123))
* **release:** enable commit-analyzer ([d8a43aa](d8a43aa))
* **release:** inherit the configuration from the shared preset ([#134](#134)) ([42fa1b4](42fa1b4))

### Performance improvements

* replace LinkedList and Optionals with ArrayDeque and EnumMap ([#166](#166)) ([b265ca6](b265ca6))
@github-actions
Copy link
Copy Markdown

🎉 This PR is included in version 0.1.8 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants