File reader with repetitions; Correct cmake configuration with QThread library local install; Simplify add_port recursion helper and use unordered_set for kernelkeeper #161

BamboWu · 2022-06-17T23:56:05Z

## Description

In order to adjust benchmark working size without bringing huge files, we let filereader kernel optionally take an argument, repetitions, which is the number of times a file is read repeatedly to feed the dataflow graph with arbitrary data to process.
The QThread include path was added with leading -I and the compiling options generated by cmake would be collapsed (missing space between two -I) making include directories invalid.

Type of change

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
This change requires a documentation update

How Has This Been Tested?

Changes are individually tested with the word search example.

Runs locally on Windows
Runs locally on Linux
Runs locally on OS X

Details

QThread library is compiled and installed at /bambo/qthread_install.

RaftLib build is configured with the cmake command:

$ QTHREAD_PATH=/bambo/qthread_install/ cmake -DCMAKE_INSTALL_PREFIX=/bambo/raft_install ..

Then make install.

To compile the search example:

$ export PKG_CONFIG_PATH=/mnt/ampere3/bambo/bmkvl/raft_install/pkgconfig/:$PKG_CONFIG_PATH
$ g++ -o search -DQTHREAD_FOUND $(pkg-config --cflags raftlib) ../examples/cppalgo/search/bmh.cpp $(pkg-config --libs raftlib)

To run the search example:

$ ./search ../examples/cppalgo/search/alice.txt
# or with 2 repetitions
$ ./search ../examples/cppalgo/search/alice.txt 2

Checklist:

My code follows the style guidelines of this project
I have performed a self-review of my own code
I have commented my code, particularly in hard-to-understand areas
I have made corresponding changes to the documentation
My changes generate no new warnings
I have added tests that prove my fix is effective or that my feature works
New and existing unit tests pass locally with my changes
Any dependent changes have been merged and published in downstream modules
I have checked my code and corrected any misspellings

Simplify the port_helper struct that was used for recursively adding ports. Two template helper functions should be neat. Change kernelkeeper to use unordered_set instead of set should make it more scalable (O(logn) access time reduced to O(1) access time).

There is a dependency between class kpair and kernel, so their method implementations have to stay in a source file. By introducing the meta data structure KernelPortMeta, kpair avoids to invoke the kernel method directly. Instead, kernel populates the KernelPortMeta structure ahead and kpair gets kernel pointer, port name, in/out port number from KernelPortMeta. This way, the mutual dependency between kpair and kernel is simplified to be kernel depends on kpair.

There was a minor bug when compile with BENCHMARK flag defined and this commit fixes it. Exposing libut uthread pointer helps us to trace the task scheduling.

Use unordered_map could lower the portmap_t access time complexity. The size of dst_kernels could be immediately taken to set waitgroup.

The support for non-string port name was broken due to some bugs fixed in this commit. Now to switch using compiler hashed port names, set `-DSTRING_NAMES=0` for cmake command. Please note that the compiler must support C++20 to have this feature.

… the test cases, will probably just make a sed script or something to convert them

This commit changes the logic to increment the blocked counter, so that the fraction function can give the real fraction to reflect the blocked ratio. Thus, dynalloc could have its condition satisfied to do resize. Another bug fixed by this commit is for those non-trivially-copyable data types such as std::string, should mark the FIFOs not resizable and avoid memcpy() those data and get undefined behaviors.

The length_signal should be sizeof( Signal ) * max_cap, otherwise in resizing, the memcpy() for signal buffer could corrupt memory. Export setPtrMap(), setPtrSet(), setInPeekSet(), setOutPeekSet() so they can be called from benchmarks.

mr-j0nes

Wow, so many changes here. I get no error when compiling the library on my end, but I do get errors when compiling the examples.

What I do is:

cd build
cmake -DBUILD_EXAMPLES=Yes ..
make

Also I do get errors when trying to compile my software with this version, here some of the warnings and errors:

/develop/sandbox/thread/RaftLib/./raftinc/defs.hpp:110:5: error: ‘raft::port_key_type operator""_port(const char*,
 std::size_t)’ defined but not used [-Werror=unused-function]                                                                 
  110 |     operator""_port( const char *input, std::size_t len )

/develop/myapp/src/StreamBuilder.cpp:211:62: error: no match for ‘operator>>’ (opera
nd types are ‘KernelPortMeta’ and ‘KernelPortMeta’)

Is the backward compatibility intended to be preserved with these changes?

jonathan-beard · 2023-06-09T10:17:46Z

I think the original intent is to provides backwards/forwards compatibility. I should be able to compile these over the weekend to help out. The _port syntax should provide a compile-time numerical mapping to the string that is unique, and therefore faster port lookup (assuming no hash collisions). Definitely lots of changes, most for the better (and bug fixes), but I definitely don't want to break old apps while doing so.

jonathan-beard · 2023-06-09T10:26:37Z

CMakeLists.txt

 find_package( QThreads )
+find_package( UT )


We probably need to make sure we add documentation for this one, as in what it is, where to get it, and how to compile + limitations. There's definitely advantages over QThreads.

jonathan-beard · 2023-06-09T10:30:18Z

examples/cppalgo/search/bmh.cpp

-    fr   read( argv[ 1 ], (fr::offset_type) term.length(), 1 );
+    if( argc > 2 )
+    {
+        repetitions = atoi( argv[ 2 ] );


We should probably use the included cmdargs package for the examples, I'll see if I can make the change for this one, then push to the same pull request. @jonathan-beard

jonathan-beard · 2023-06-09T10:30:48Z

examples/cppalgo/search/bmh.cpp

+    {
+        repetitions = atoi( argv[ 2 ] );
+    }
+    fr   read( argv[ 1 ], (fr::offset_type) term.length(), 1, repetitions );


same for this one, parse with cmd-args so we get a menu for the example then use the assigned variable here.

jonathan-beard · 2023-06-09T10:31:48Z

raftinc/allocate.hpp

 * all the currently allocated FIFO objects within the streaming
 * graph.  This is primarily for instrumentation puposes.
 * @author: Jonathan Beard
 * @version: Tue Sep 16 20:20:06 2014
- * 
+ *


@BamboWu , did UT (or yourself) want to also add yourself to the author line?

This one should be fine, I think.
The most of modifications I did is on another branch armq_dev, and I added UT in the header there.

jonathan-beard · 2023-06-09T10:33:58Z

raftinc/allocate.hpp

+    {
+        for( FIFO *f : allocated_fifo )
+        {
+#if RAFT_DUMP_STATS


@jonathan-beard , find a better solution for this. Perhaps define a shared memory hook that is automatically dumped to if a telemetry agent is attached?

jonathan-beard · 2023-06-09T10:36:02Z

raftinc/allocate.hpp

+        src->setFIFO( fifo );
+        dst->setFIFO( fifo );
+        /** NOTE: this list simply speeds up the monitoring if we want it **/
+        allocated_fifo.insert( fifo );


Need to double check that the monitoring thread (if it's used) still traverses this structure (e.g., to dump periodic frames of queue stats.

jonathan-beard · 2023-06-09T10:38:00Z

raftinc/basicparallel.hpp

-      stream << "service rate: " << s.service_rate << "\n";
-      return( stream );
-   }
+    std::uint16_t occ_in  = 0;


@jonathan-beard , double check to see if the service rate instrumentation and such is still functional. I think @BamboWu was primarily using the enqueue/dequeue blocked stats, would be nice to have the service rate estimates going again for initial transport buffer sizing.

BamboWu added 8 commits June 16, 2022 21:40

Correct QThread library config

74b1f00

Let file reader read a file repeatedly

9516437

WIP Convert C++ source code to template headers

d4e6c78

Substitute yield function by cmake

dd4f673

Merge: Fix race condition caused by OoO stores

a643a35

Add the option to use libut for threading

c9ec907

BamboWu force-pushed the ut_dev branch 2 times, most recently from d010b28 to 88bc40d Compare February 7, 2023 03:05

Add some missing stream operator functions

6eb412a

BamboWu force-pushed the ut_dev branch from 88bc40d to 6eb412a Compare February 8, 2023 17:17

BamboWu and others added 9 commits February 18, 2023 18:26

Get BENCHMARK flag work and expose libut uthread

52519c5

There was a minor bug when compile with BENCHMARK flag defined and this commit fixes it. Exposing libut uthread pointer helps us to trace the task scheduling.

Hash table for portmap_t, set waitgroup easier

4cabbd3

Use unordered_map could lower the portmap_t access time complexity. The size of dst_kernels could be immediately taken to set waitgroup.

Enable non-string port name

4360e67

The support for non-string port name was broken due to some bugs fixed in this commit. Now to switch using compiler hashed port names, set `-DSTRING_NAMES=0` for cmake command. Please note that the compiler must support C++20 to have this feature.

fixed the no stringname commit for qinzhe, need to update the rest of…

aca9dd4

… the test cases, will probably just make a sed script or something to convert them

Merge nostrnm, the fix for STRING_NAMES=0

49703e4

Correct blocked ratio calculation, update pointers

9853d07

Dump stats

b60d623

Correct length_signal, export setPtrMap() ...

71b1fa8

The length_signal should be sizeof( Signal ) * max_cap, otherwise in resizing, the memcpy() for signal buffer could corrupt memory. Export setPtrMap(), setPtrSet(), setInPeekSet(), setOutPeekSet() so they can be called from benchmarks.

jonathan-beard assigned jonathan-beard and mr-j0nes Jun 2, 2023

jonathan-beard added the enhancement label Jun 2, 2023

jonathan-beard assigned BamboWu Jun 2, 2023

jonathan-beard requested review from pl53 and mr-j0nes June 2, 2023 11:00

jonathan-beard removed their assignment Jun 2, 2023

jonathan-beard self-requested a review June 3, 2023 12:52

mr-j0nes reviewed Jun 8, 2023

View reviewed changes

jonathan-beard reviewed Jun 9, 2023

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

File reader with repetitions; Correct cmake configuration with QThread library local install; Simplify add_port recursion helper and use unordered_set for kernelkeeper #161

File reader with repetitions; Correct cmake configuration with QThread library local install; Simplify add_port recursion helper and use unordered_set for kernelkeeper #161

BamboWu commented Jun 17, 2022 •

edited

mr-j0nes left a comment

jonathan-beard commented Jun 9, 2023

jonathan-beard Jun 9, 2023

jonathan-beard Jun 9, 2023

jonathan-beard Jun 9, 2023

jonathan-beard Jun 9, 2023

BamboWu Jun 9, 2023

jonathan-beard Jun 9, 2023

jonathan-beard Jun 9, 2023

jonathan-beard Jun 9, 2023

File reader with repetitions; Correct cmake configuration with QThread library local install; Simplify add_port recursion helper and use unordered_set for kernelkeeper #161

Are you sure you want to change the base?

File reader with repetitions; Correct cmake configuration with QThread library local install; Simplify add_port recursion helper and use unordered_set for kernelkeeper #161

Conversation

BamboWu commented Jun 17, 2022 • edited

Type of change

How Has This Been Tested?

Details

Checklist:

mr-j0nes left a comment

Choose a reason for hiding this comment

jonathan-beard commented Jun 9, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

BamboWu commented Jun 17, 2022 •

edited