[WIP]: MicroMap #1208

mvuksano · 2014-07-07T13:24:22Z

An implementation of the Map which initially uses array and after a number of elements is inserted switches to a HashMap implementation.

MicroMap is intended to be used when one expects that the number of elements will be quite low. In this setting it's better to use an array and use linear search to locate elements rather then using the actual HashMap. After a certain number of elements have been inserted HashMap will perform better then array and linear search.

Worst case scenario will be when the number of elements keeps oscillating around threshold value which will cause the map to keep switching from array to HashMap (and vice versa). Switching form array to hashMap and the other way around is an expensive operation.

mvuksano · 2014-07-07T13:25:10Z

@mhevery - for review.

Still need to add docs though - at least class description and why this map exists in the first place...

jbdeboer · 2014-07-07T13:36:58Z

Great! Do you have benchmarks?

jbdeboer · 2014-07-07T13:41:55Z

lib/collection/micro_map.dart

+
+  void _copyAllElementsToMap() {
+    var i = 0;
+    if (delegate==null) delegate = {};


{} are known to be slow, the type of the delegate should be configurable.

jbdeboer · 2014-07-07T13:44:13Z

lib/collection/micro_map.dart

+
+  bool containsKey(Object key) {
+    if (elements != null) {
+      for(var i=0; i<elements.length; i++) {


This should read i<count, otherwise you are iterating unnecessarily over nulls.

mvuksano · 2014-07-07T14:11:01Z

@jbdeboer Added a benchmark.. but the results are not promissing. HashMap still beat all other types of maps...

HashMapRead(RunTime): 17.83071518998627 us.
FastMapRead(RunTime): 41.24731892426992 us.
MicroMapRead(RunTime): 50.60984867655246 us.

mvuksano · 2014-07-07T14:31:46Z

Actually, after compiling and running in JS things look a bit different. But still MicroMap does not beat FastMap. I think I should try getting rid of _KeyValue object and use two arrays. The other option is to use literally 40 variables as in FastMap. Thoughts? @mhevery @jbdeboer

mvuksano · 2014-07-07T15:00:27Z

I'm also thinking of chaning the strategy with this arrays. I can try keeping them sorted so lookups could be a lot faster....

mhevery · 2014-07-08T03:59:41Z

@markovuksanovic I am sorry for not making the request clearer. A list will never perform as fast as a field. What I am looking for is 20 field keys and 20 field values, just as they were in my original benchmarks. (It needs to implement Map) This is fast for several reasons. 1) field access is relatively fast, 2) fields are located next to each other in memory layout therefore they will have high cache hit rate. Yes, it feels a bit strange to have 40 fields, but compared to the cost of map with 1 key it is still better since Map has to have several objects List and Buckets to scale, which add up to way than 40 fields.

mvuksano · 2014-07-08T13:30:16Z

@mhevery I've refactored this to use fields and the results are noticeable now.

mhevery · 2014-07-08T18:42:26Z

benchmark/hashmap.dart

+  }
+}
+
+class FastMap<K,V> implements Map {


Delete the FastMap from the benchmark.

mhevery · 2014-07-08T19:11:02Z

Nice work. Please take care of the comments.

What are your benchmark numbers. How much faster are we?

…ay and after a number of elements is inserted switches to a HashMap implementation. MicroMap is intended to be used when one expects that the number of elements will be quite low. In this setting it's better to use an array and use linear search to locate elements rather then using the actual HashMap. After a certain number of elements have been inserted HashMap will perform better then array and linear search. Worst case scenario will be when the number of elements keeps oscillating around threshold value which will cause the map to keep switching from array to HashMap (and vice versa). Switching form array to hashMap and the other way around is an expensive operation. Closes dart-archive#1201

mvuksano · 2014-07-11T14:10:35Z

@rkirov I'm considering that option too... I worked on this today and I think I'll need a couple of more iterables (e.g. ListIterable, SubList iterable, whileTakeItrable, MappedIterable,...) I used internal implemnations in dart as a reference for MicroIterable. Turns out implementing iterable is a bit more challenging then I initially thought :)

mvuksano · 2014-07-11T14:26:38Z

Actually, I got it working... The code should be fine now, although I need to write some more tests....

vicb · 2014-07-14T08:32:03Z

lib/collection/micro_iterable.dart

+
+import 'dart:collection';
+
+const int _LIST_ELEMENTS = 20;


vicb · 2014-07-14T09:15:47Z

@markovuksanovic @mhevery I'm a bit concerned that the lack null support might trigger subtle bugs that could be hard to debug. Those bug might be intermittent because a) null values might be intermittent and b) the behavior depends on the map size.

We should investigate the perf impact for supporting null values and at least add assertions

rkirov · 2014-07-14T22:04:50Z

We can continue the discussion on #1230. It is based on this PR and has a few extra features - supporting null keys, doesn't go back to fields mode once map mode is triggered. Also it replaces HashMap at a few key places in the framework.

I think iterators need more work (they should be views into underlying micromap, instead of holding their own copies of the data).

jbdeboer reviewed Jul 7, 2014
View reviewed changes

mvuksano added cla: yes and removed cla: no labels Jul 7, 2014

jbdeboer reviewed Jul 7, 2014
View reviewed changes

mhevery reviewed Jul 8, 2014
View reviewed changes

benchmark/hashmap.dart

}

}

class FastMap<K,V> implements Map {

Copy link

Contributor

mhevery Jul 8, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Delete the FastMap from the benchmark.

mhevery assigned rkirov Jul 9, 2014

mvuksano added 3 commits July 10, 2014 22:42

First pass of MicroIterable

c7fc8f3

Added tests stub

3b08e46

First test

62cb43b

MicroIterable

f24a42c

vicb reviewed Jul 14, 2014
View reviewed changes

lib/collection/micro_iterable.dart

import 'dart:collection';

const int _LIST_ELEMENTS = 20;

Copy link

Contributor

vicb Jul 14, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

unused ?

A few more tests

9169c52

rkirov mentioned this pull request Jul 14, 2014

[WIP] Feature/micromap #1230

Closed

mhevery closed this Jul 21, 2014

IgorMinar unassigned rkirov Oct 3, 2017

[WIP]: MicroMap #1208

[WIP]: MicroMap #1208

Uh oh!

Conversation

mvuksano commented Jul 7, 2014

Uh oh!

mvuksano commented Jul 7, 2014

Uh oh!

jbdeboer commented Jul 7, 2014

Uh oh!

jbdeboer Jul 7, 2014

Choose a reason for hiding this comment

Uh oh!

jbdeboer Jul 7, 2014

Choose a reason for hiding this comment

Uh oh!

mvuksano commented Jul 7, 2014

Uh oh!

mvuksano commented Jul 7, 2014

Uh oh!

mvuksano commented Jul 7, 2014

Uh oh!

mhevery commented Jul 8, 2014

Uh oh!

mvuksano commented Jul 8, 2014

Uh oh!

mhevery Jul 8, 2014

Choose a reason for hiding this comment

Uh oh!

mhevery commented Jul 8, 2014

Uh oh!

mvuksano commented Jul 11, 2014

Uh oh!

mvuksano commented Jul 11, 2014

Uh oh!

vicb Jul 14, 2014

Choose a reason for hiding this comment

Uh oh!

vicb commented Jul 14, 2014

Uh oh!

rkirov commented Jul 14, 2014

Uh oh!

Uh oh!