Try to enhance performance of FNT(257) #264

lamphamsy · 2018-11-02T15:48:56Z

It focuses on improving performance of FNT(257)
Use two vectors instead of unordered_map: one vector for locations, the other for marked values.
I tried to use reserve() for unordered_map`, it improves about 10% for encoding
The use of two vectors improves 1.5-2x on encoding speeds. And a significant enhancement is observed for decoding.

src/property.cpp

src/property.h

slaperche-scality · 2019-02-11T07:15:02Z

src/property.h

-        for (auto& kv : props) {
-            dwords[i++] = htonl(static_cast<uint32_t>(kv.first));
+        for (auto const& key : keys) {
+            dwords[i++] = htonl(static_cast<uint32_t>(key));
        }
        dwords[1] = htonl(i - 2);
        for (; i < n_dwords; i++) {


again not your code, but 0 has the same representation in little endian and big endian, so why not using a fill of fill_n instead of a manual loop here?

okay, I will update it

use fill

slaperche-scality · 2019-02-11T08:36:43Z

src/property.h

@@ -141,6 +142,63 @@ class FntProperties : public Properties {
  public:
 };

+/** Iterator on Properties


Good idea to add an iterator to Properties 🙂

But that's not how you implement an iterator in C++.

See:

https://stackoverflow.com/questions/7758580/writing-your-own-stl-container/7759622#7759622 (an example)

http://www.cplusplus.com/reference/iterator/ForwardIterator/ (to see the expected interface)

https://www.fluentcpp.com/2018/05/08/std-iterator-deprecated/

Note that they're usually implemented as a nested class (nested in the container).
In think in our use case, implementing a forward iterator should be enough.

Note that by using a std::pair as value_type, you can even keep the code that was used with unordered_map (with first and second).

And once you have your iterator class, you should add the begin and endmethods toProperties`.
Once you have those two, it's very easy to provide the whole interface.

iterator begin() noexcept { […] } const_iterator begin() const noexcept { […] } iterator end() noexcept { […] } const_iterator end() const noexcept { […] } const_iterator cbegin() const noexcept { return begin(); } const_iterator cend() const noexcept { return end(); }

Ok, I will try to improve the iterator.

I checked that unordered_map or map don't give good perf for both of encoding (insert) and decoding (access). Vector is a better approach IMHO.
We can find a comparison between them, e.g. http://john-ahlgren.blogspot.com/2013/10/stl-container-performance.html

src/fec_context.h

src/property.h

lamphamsy

Thanks so much for your reviews

src/fec_context.h

src/property.cpp

src/property.h

lamphamsy · 2019-02-13T14:18:38Z

src/property.h

-        for (auto& kv : props) {
-            dwords[i++] = htonl(static_cast<uint32_t>(kv.first));
+        for (auto const& key : keys) {
+            dwords[i++] = htonl(static_cast<uint32_t>(key));


Yes, we should have a better solution for fnt_serialize. Could you create an issue for it?

use narrow_cast

lamphamsy · 2019-02-13T14:19:23Z

src/property.h

-        for (auto& kv : props) {
-            dwords[i++] = htonl(static_cast<uint32_t>(kv.first));
+        for (auto const& key : keys) {
+            dwords[i++] = htonl(static_cast<uint32_t>(key));
        }
        dwords[1] = htonl(i - 2);
        for (; i < n_dwords; i++) {


okay, I will update it

use fill

src/property.h

lamphamsy · 2019-02-13T14:30:58Z

src/property.h

@@ -141,6 +142,63 @@ class FntProperties : public Properties {
  public:
 };

+/** Iterator on Properties


Ok, I will try to improve the iterator.

I checked that unordered_map or map don't give good perf for both of encoding (insert) and decoding (access). Vector is a better approach IMHO.
We can find a comparison between them, e.g. http://john-ahlgren.blogspot.com/2013/10/stl-container-performance.html

src/property.h

We use std::vector to store pairs of location and marker. It gives better performance compared to map or unordered_map for our operations: addition & accessing. Util functions are added: - location(i): get location of the ith element - marker(i): get marker of the ith element - is_marked(i, loc): check if location of the ith element == loc - in_range(i, min, max): check if location of the ith element is within the range [min, max) Note, in these util functions, we assume that stored elements are sorted in ascending order of location.

Each property will be sort Add vector of indices that stores index of the accessing currently element of Property

- As `props_indices` in `DecodeContext` will be updated during decoding, we remove `const` keyword in declarations of `DecodeContext` and `Properties` - We note that the locations are retrieved in the ascending order according to the decoding process. Hence we implemented util functions in Properties to retrieve location of symbols whose value = `q-1`.

- As `props_indices` in `DecodeContext` will be updated during decoding, we remove `const` keyword in declarations of `DecodeContext` and `Properties` - Use util functions in Properties to retrieve location of symbols whose value = `q-1`.

vrancurel added the Release 0.1 Initial Release label Dec 19, 2018

lamphamsy force-pushed the eh/props branch from 18ca3ef to 121ad8f Compare December 20, 2018 15:53

lamphamsy requested a review from slaperche-scality December 20, 2018 15:53

lamphamsy force-pushed the eh/props branch 3 times, most recently from 4233ae6 to c4e704a Compare February 8, 2019 13:13

slaperche-scality suggested changes Feb 11, 2019

View reviewed changes

lamphamsy commented Feb 13, 2019

View reviewed changes

lamphamsy force-pushed the eh/props branch 2 times, most recently from ccf58c0 to 3da1a26 Compare February 20, 2019 13:34

lamphamsy force-pushed the eh/props branch from 3da1a26 to ec36960 Compare March 15, 2019 16:04

lamphamsy added 6 commits March 19, 2019 17:01

Property in FecContext: sort map & add vector of indices

37dbd25

Each property will be sort Add vector of indices that stores index of the accessing currently element of Property

Fec utest: update for new initilization step of context

673fa6c

Update benchmark script focusing on FNT only

22561b8

lamphamsy force-pushed the eh/props branch from ec36960 to 22561b8 Compare March 19, 2019 16:01

slaperche-scality approved these changes Mar 21, 2019

View reviewed changes

lamphamsy merged commit 6b465d4 into master Mar 21, 2019

lamphamsy deleted the eh/props branch March 21, 2019 15:33

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Try to enhance performance of FNT(257) #264

Try to enhance performance of FNT(257) #264

lamphamsy commented Nov 2, 2018

slaperche-scality Feb 11, 2019

lamphamsy Feb 13, 2019 •

edited

Loading

slaperche-scality Feb 11, 2019

lamphamsy Feb 13, 2019

lamphamsy left a comment

lamphamsy Feb 13, 2019 •

edited

Loading

lamphamsy Feb 13, 2019 •

edited

Loading

lamphamsy Feb 13, 2019

Try to enhance performance of FNT(257) #264

Try to enhance performance of FNT(257) #264

Conversation

lamphamsy commented Nov 2, 2018

slaperche-scality Feb 11, 2019

Choose a reason for hiding this comment

lamphamsy Feb 13, 2019 • edited Loading

Choose a reason for hiding this comment

slaperche-scality Feb 11, 2019

Choose a reason for hiding this comment

lamphamsy Feb 13, 2019

Choose a reason for hiding this comment

lamphamsy left a comment

Choose a reason for hiding this comment

lamphamsy Feb 13, 2019 • edited Loading

Choose a reason for hiding this comment

lamphamsy Feb 13, 2019 • edited Loading

Choose a reason for hiding this comment

lamphamsy Feb 13, 2019

Choose a reason for hiding this comment

lamphamsy Feb 13, 2019 •

edited

Loading

lamphamsy Feb 13, 2019 •

edited

Loading

lamphamsy Feb 13, 2019 •

edited

Loading