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

Optimize ColorMap #1648

Merged
merged 10 commits into from
Oct 5, 2016
Merged

Optimize ColorMap #1648

merged 10 commits into from
Oct 5, 2016

Conversation

fosskers
Copy link
Contributor

@fosskers fosskers commented Oct 3, 2016

TODO

Let: k be the number of pixels to colour, and n the number of colour breaks, where usually k > n by several orders of magnitude

  • Use a TrieMap to memoize IntColorMap.map results
    • Worst case (all unique pixel values): O(kn)
    • Best case (all pixels same value): O(k + n)
  • Write BTree[T]
  • Use a custom BTree for DoubleColorMap
    • Via B-tree: O(nlogn + klogn) == O((n + k)logn) ~= O(klogn)
  • Fix tests
  • BTree.pretty should return String
  • Use BTree for IntColorMap

Motivation

This addresses #1489 . Currently every call to IntColorMap.map or DoubleColorMap.map performs a O(n) search for the correct colour ramp value, given some raster's pixel value. The total cost to colour a raster is then O(kn).

Next Steps

Use Spire to generalize over Int and Double?

- Memoize the results of `map` in a `TrieMap` for faster subsequent lookups.
A more involved custom binary search could be employed for faster lookups in
`DoubleColorMap`, as it can't take advantage of `TrieMap` in the same way.
- A generalized binary search with a custom "target test" predicate.
- Also, `BTree` can print itself now. Should convert to returning a `String`
instead.
@fosskers fosskers changed the title [WIP] Optimize ColorMap Optimize ColorMap Oct 5, 2016
@fosskers
Copy link
Contributor Author

fosskers commented Oct 5, 2016

Bonus, BTrees have a nice string representation:

└── 4
    ├── 6
       ├── 7
          ├── 
          └── 
       └── 5
           ├── 
           └── 
    └── 2
        ├── 3
           ├── 
           └── 
        └── 1
            ├── 
            └── 

@fosskers
Copy link
Contributor Author

fosskers commented Oct 5, 2016

Don't merge yet: I still need to run benchmarks.

@fosskers
Copy link
Contributor Author

fosskers commented Oct 5, 2016

1280x1280 pixel Tile, 50 Breaks, 5 trials each, same JVM

 ORIGINAL LINEAR
 ---------------
 Rendering => 176.53 ms
 Rendering => 180.56 ms
 Rendering => 170.98 ms
 Rendering => 170.94 ms
 Rendering => 170.47 ms

 Mean: 173.89 ms

 WITH BTREE
 ----------
 Rendering => 120.25 ms
 Rendering => 117.32 ms
 Rendering => 121.19 ms
 Rendering => 117.35 ms
 Rendering => 116.86 ms

 Mean: 118.6 ms

 Speedup: ~47%

@lossyrob lossyrob merged commit b88c3dc into locationtech:master Oct 5, 2016
@lossyrob lossyrob added this to the 1.0 milestone Oct 18, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants