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.

Sign up for GitHub

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

Jump to bottom

Optimize array access and add ability to hint array size #455

Merged

sebastienros merged 8 commits into sebastienros:dev from lahma:perf/optimize-arrays

Jan 2, 2018

Collaborator

lahma commented Dec 22, 2017 •

edited

Made it possible to hint about wanted array size
Support for dense arrays
MruPropertyCache is lazy

ArrayBenchmark

Baseline

Method	N	Mean	Error	StdDev	Gen 0	Allocated
Slice	100	2.606 ms	0.0555 ms	0.0519 ms	546.8750	2.2 MB
Concat	100	2.758 ms	0.0231 ms	0.0205 ms	593.7500	2.38 MB
Unshift	100	107.151 ms	0.3361 ms	0.2807 ms	8250.0000	33.16 MB
Push	100	33.993 ms	0.6668 ms	0.7134 ms	4000.0000	16.13 MB
Index	100	26.968 ms	0.0660 ms	0.0585 ms	2750.0000	11.07 MB
Map	100	12.291 ms	0.0380 ms	0.0356 ms	3546.8750	14.23 MB
Apply	100	5.936 ms	0.0071 ms	0.0063 ms	742.1875	2.97 MB
JsonStringifyParse	100	11.847 ms	0.0464 ms	0.0434 ms	2046.8750	8.22 MB

After sparse array optimization

Method	N	Mean	Error	StdDev	Gen 0	Allocated
Slice	100	1.149 ms	0.0152 ms	0.0134 ms	382.8125	1.53 MB
Concat	100	1.305 ms	0.0027 ms	0.0022 ms	457.0313	1.83 MB
Unshift	100	46.830 ms	0.0509 ms	0.0451 ms	8125.0000	32.51 MB
Push	100	33.087 ms	0.1223 ms	0.1144 ms	3937.5000	15.81 MB
Index	100	26.781 ms	0.0412 ms	0.0344 ms	2593.7500	10.45 MB
Map	100	8.557 ms	0.0126 ms	0.0105 ms	2953.1250	11.87 MB
Apply	100	5.607 ms	0.0047 ms	0.0037 ms	718.7500	2.9 MB
JsonStringifyParse	100	11.445 ms	0.0234 ms	0.0219 ms	1984.3750	7.95 MB

After introducing dense array

Method	N	Mean	Error	StdDev	Gen 0	Allocated
Slice	100	935.5 us	3.443 us	3.052 us	336.9141	1.35 MB
Concat	100	1,068.9 us	2.023 us	1.892 us	361.3281	1.45 MB
Unshift	100	41,124.9 us	116.609 us	103.371 us	8687.5000	34.79 MB
Push	100	28,129.1 us	77.340 us	72.344 us	3625.0000	14.53 MB
Index	100	25,813.5 us	84.770 us	79.294 us	2500.0000	10.07 MB
Map	100	8,320.5 us	33.465 us	27.945 us	2906.2500	11.68 MB
Apply	100	1,160.1 us	11.889 us	11.121 us	369.1406	1.48 MB
JsonStringifyParse	100	6,885.8 us	18.351 us	17.166 us	1671.8750	6.71 MB

ArrayStressBenchmark

Baseline

Method	N	Mean	Error	StdDev	Gen 0	Gen 1	Gen 2	Allocated
Jint	20	2.520 s	0.0885 s	0.0050 s	223750.0000	23250.0000	11500.0000	979.52 MB

After sparse array optimization

Method	N	Mean	Error	StdDev	Gen 0	Gen 1	Gen 2	Allocated
Jint	20	1.492 s	0.0479 s	0.0027 s	180000.0000	20000.0000	9750.0000	805.61 MB

After introducing dense array

Method	N	Mean	Error	StdDev	Gen 0	Gen 1	Gen 2	Allocated
Jint	20	2.096 s	0.0750 s	0.0042 s	198416.6667	8833.3333	3833.3333	873.57 MB

UncacheableExpressionsBenchmark

Baseline

Method	N	Mean	Error	StdDev	Gen 0	Gen 1	Allocated
Benchmark	500	1.062 s	0.0289 s	0.0406 s	202687.5000	55312.5000	903.2 MB

After sparse array optimization

Method	N	Mean	Error	StdDev	Gen 0	Gen 1	Allocated
Benchmark	500	845.1 ms	3.704 ms	5.430 ms	178058.3333	45079.1667	783.57 MB

After introducing dense array

Method	N	Mean	Error	StdDev	Gen 0	Gen 1	Allocated
Benchmark	500	797.6 ms	2.303 ms	3.447 ms	189070.8333	2366.6667	761.26 MB

ayende reviewed

View reviewed changes

Jint/Native/Array/ArrayPrototype.cs Outdated

    
            @@ -79,11 +79,9 @@ private JsValue LastIndexOf(JsValue thisObj, JsValue[] arguments)
          
                          for (; k >= 0; k--)

                          {

                              var kString = k.ToString();

Contributor

ayende Dec 23, 2017

Change k to int and use TypeConverter.ToString(k); ? Should save the allocation and the string gen for most cases.

Collaborator Author

lahma Dec 23, 2017

I think I had it there but some test gave overflow, might just have needed uint or something. Worth to check again.

Jint/Native/Array/ArrayPrototype.cs Outdated

    
            @@ -170,14 +165,16 @@ private JsValue Filter(JsValue thisObj, JsValue[] arguments)
          
                          var a = (ArrayInstance)Engine.Array.Construct(Arguments.Empty);

                          var to = 0;

                          var jsValues = new JsValue[3];

Contributor

ayende Dec 23, 2017

Cache that as local thread stuff as well?
For that matter, might as well create some shared cache per thread for all of that and have a property such as List, 3 element array, etc.

Collaborator Author

lahma Dec 23, 2017

Before I do own benchmarks, would you happen to know performance benefits between small array allocation and thread local data access? What I've understood small arrays are quite efficient, but probably worth checking out.

Jint/Native/Array/ArrayPrototype.cs Outdated

    
                          var a = Engine.Array.Construct(new JsValue[] {len});

                          var a = Engine.Array.Construct(new JsValue[] {len}, len);

                          var jsValues = new JsValue[3];

Contributor

ayende Dec 23, 2017

Same

Jint/Native/Array/ArrayPrototype.cs Outdated

    
            @@ -234,14 +232,16 @@ private JsValue ForEach(JsValue thisObj, JsValue[] arguments)
          
                              throw new JavaScriptException(Engine.TypeError, "Argument must be callable");

                          });

                          var jsValues = new JsValue[3];

Contributor

ayende Dec 23, 2017

Same

Jint/Native/Array/ArrayPrototype.cs Outdated

    
            @@ -262,14 +262,16 @@ private JsValue Some(JsValue thisObj, JsValue[] arguments)
          
                              throw new JavaScriptException(Engine.TypeError, "Argument must be callable");

                          });

                          var jsValues = new JsValue[3];

Contributor

ayende Dec 23, 2017

Same

Jint/Native/Array/ArrayPrototype.cs Outdated

    
            @@ -294,14 +296,16 @@ private JsValue Every(JsValue thisObj, JsValue[] arguments)
          
                              throw new JavaScriptException(Engine.TypeError, "Argument must be callable");

                          });

                          var jsValues = new JsValue[3];

Contributor

ayende Dec 23, 2017

Same

Jint/Native/Array/ArrayPrototype.cs Outdated

    
            @@ -344,10 +348,8 @@ private JsValue IndexOf(JsValue thisObj, JsValue[] arguments)
          
                          for (; k < len; k++)

                          {

                              var kString = k.ToString();

Contributor

ayende Dec 23, 2017

Same

lahma force-pushed the perf/optimize-arrays branch 2 times, most recently from db116fa to e931502 Compare

December 24, 2017 09:00

Collaborator Author

lahma commented Dec 24, 2017

I've implemented @ayende 's suggestions. I also updated the before and after numbers above, now rebased based on current dev.

lahma force-pushed the perf/optimize-arrays branch from 37bed59 to c92046e Compare

December 27, 2017 11:24

Collaborator Author

lahma commented Dec 27, 2017 •

edited

I think I've done most I can here now for sparse array case, you can see updated numbers above. To proceed with this we need to process other PRs first, which should also give perf boost here too.

lahma force-pushed the perf/optimize-arrays branch 4 times, most recently from afcdf78 to 5e82122 Compare

December 31, 2017 09:46

lahma changed the title ~~WIP Optimize array access and add ability to hint array size~~ Optimize array access and add ability to hint array size

Collaborator Author

lahma commented Dec 31, 2017

I have implemented the dense array thingie. This helped to squeeze some more speed. ArrayStressBenchmark had negative change due to dense array but it's still faster than before. I guess the unrealistic test case just works better with dictionary based backend.

Owner

sebastienros commented Dec 31, 2017

That's awesome! I am very slow on reviews and merges because of the holidays, but should be back on track in two days.

Collaborator Author

lahma commented Dec 31, 2017

No worries, have a great new year!

lahma added 4 commits

January 2, 2018 21:40


          sebastienros#451 optimize array access and add ability to hint array …

05f9482

…size


          review fixes

04fdbc9

* introduce ArrayExecutionContext for thread-local call data
* use TypeConverter.ToString for rest of indexes too


          use direct uint access to arrays in functions

13295a2

* harmonize xUnit versions


          implement dense array, add more capacity hints when constructing

45bb138

lahma force-pushed the perf/optimize-arrays branch from 5e82122 to 45bb138 Compare

January 2, 2018 19:44

sebastienros reviewed

View reviewed changes

Jint/Native/Array/ArrayInstance.cs

-                              yield return new KeyValuePair<string, PropertyDescriptor>(TypeConverter.ToString(entry.Key), entry.Value);
+                              for (var i = 0; i < _dense.Length; i++)
+                              {
+                                  if (_dense[i] != null)

Owner

sebastienros Jan 2, 2018

If it's dense, shouldn't the first null value mark that all subsequent ones are not used and break the loop?

Collaborator Author

lahma Jan 2, 2018

It's expected to be dense, but allows sparseness as currently shares the ArrayProtype API. So you could call delete in the middle if I remember right. Ideally we could natively support unshift with moving start index for array. But this requires some work and moving operations closer to arrayinstance.

lahma and others added 4 commits

January 2, 2018 23:56


          fix bad rebase

1d3dcf9


          cosmetics: fix indents, return MruPropertyCaches as internal

bfb1dbe


          Merge branch 'dev' into perf/optimize-arrays


          Merge branch 'dev' into perf/optimize-arrays

67424c9

sebastienros merged commit 98dc732 into sebastienros:dev

lahma deleted the perf/optimize-arrays branch

January 3, 2018 05:43

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