UCS/API event_set #3426

hiroyuki-sato · 2019-03-28T12:37:14Z

What

This PR is aim for porting OpenUCX to macOS.
This PR a request for review design API.

Why ?

macOS doesn't exist epoll function. Instead, it needs to use kqueue.

How ?

I'm seperating epoll codes into ucs/sys/event_set.{c,h}.

@yosefe Could you review this?

swx-jenkins1 · 2019-03-28T12:37:29Z

Can one of the admins verify this patch?

src/ucs/sys/event_set.c

shamisp · 2019-03-28T14:15:44Z

ok to test

swx-jenkins1 · 2019-03-28T15:44:46Z

Test PASSed.
See http://bgate.mellanox.com/jenkins/job/gh-ucx-pr/6771/ for details.

mellanox-github · 2019-03-28T17:25:57Z

Test PASSed.
See http://hpc-master.lab.mtl.com:8080/job/hpc-ucx-pr/9560/ for details (Mellanox internal link).

src/ucs/sys/event_set.c

src/ucs/sys/event_set.h

yosefe · 2019-03-31T22:56:23Z

also, need to add unit test for it, and add it to Makefile

hiroyuki-sato · 2019-04-01T08:50:32Z

Hello, @yosefe

Thank you for the comment.
I fixed several comments. I have some questions. I'll post later.
Is ucx/test/gtest/ucs/test_* unit test file?
And Is it required to write in C++?

Best regards.

yosefe · 2019-04-01T09:00:23Z

@hiroyuki-sato yes, these are the unit test files, they have to be i C++ because it uses google test infrastructure. i'd suggest to copy/paste from another test for start, ucs tests are pretty simple

swx-jenkins1 · 2019-04-01T10:19:39Z

Test FAILed.
See http://bgate.mellanox.com/jenkins/job/gh-ucx-pr/6796/ for details.

hiroyuki-sato · 2019-04-01T10:27:14Z

Hello, @yosefe

Thanks!, I added a unit test skeleton and run test.
hiroyuki-sato@e40cf72

I'll add test cases.

GTEST_FILTER='*test_event_set*' make -C test/gtest test

mellanox-github · 2019-04-01T11:47:34Z

Test FAILed.
See http://hpc-master.lab.mtl.com:8080/job/hpc-ucx-pr/9588/ for details (Mellanox internal link).

swx-jenkins1 · 2019-04-01T13:19:02Z

Test PASSed.
See http://bgate.mellanox.com/jenkins/job/gh-ucx-pr/6800/ for details.

mellanox-github · 2019-04-01T14:49:40Z

Test FAILed.
See http://hpc-master.lab.mtl.com:8080/job/hpc-ucx-pr/9591/ for details (Mellanox internal link).

hiroyuki-sato

Hello, @yosefe

Could you review those commit?

src/ucs/sys/event_set.c

src/ucs/sys/event_set.h

test/gtest/ucs/test_event_set.cc

swx-jenkins1 · 2019-04-09T13:07:01Z

Test FAILed.
See http://bgate.mellanox.com/jenkins/job/gh-ucx-pr/6942/ for details.

mellanox-github · 2019-04-09T16:01:52Z

Test FAILed.
See http://hpc-master.lab.mtl.com:8080/job/hpc-ucx-pr/9754/ for details (Mellanox internal link).

src/ucs/sys/event_set.c

src/ucs/sys/event_set.h

test/gtest/ucs/test_event_set.cc

hiroyuki-sato · 2019-04-12T10:32:54Z

Hello, @yosefe Thank you for the comment. I'll fix it.

hiroyuki-sato · 2019-04-20T07:27:34Z

Hello, @yosefe and @shamisp.
I changed codes. Please take a look when you have time.

Currently, I created two tests.

event read test
event timeout test.

Do we need to create a write(EPOLLOUT) test?
If so, I think it needs to use two thread. One is event writer, the other is event waiter.
Do I need to use pthread_create? Do you have any idea?

swx-jenkins1 · 2019-04-20T08:25:22Z

Test PASSed.
See http://bgate.mellanox.com/jenkins/job/gh-ucx-pr/7101/ for details.

swx-jenkins1 · 2019-04-20T08:25:22Z

Test PASSed.
See http://bgate.mellanox.com/jenkins/job/gh-ucx-pr/7102/ for details.

mellanox-github · 2019-04-20T11:11:49Z

Test FAILed.
See http://hpc-master.lab.mtl.com:8080/job/hpc-ucx-pr/9943/ for details (Mellanox internal link).

mellanox-github · 2019-04-20T11:11:50Z

Test FAILed.
See http://hpc-master.lab.mtl.com:8080/job/hpc-ucx-pr/9944/ for details (Mellanox internal link).

mellanox-github · 2019-04-29T20:06:55Z

Test FAILed.
See http://hpc-master.lab.mtl.com:8080/job/hpc-ucx-pr/10061/ for details (Mellanox internal link).

mellanox-github · 2019-04-29T20:07:16Z

Test FAILed.
See http://hpc-master.lab.mtl.com:8080/job/hpc-ucx-pr/10063/ for details (Mellanox internal link).

swx-jenkins1 · 2019-04-30T03:16:31Z

Test PASSed.
See http://bgate.mellanox.com/jenkins/job/gh-ucx-pr/7205/ for details.

mellanox-github · 2019-04-30T05:36:51Z

Test FAILed.
See http://hpc-master.lab.mtl.com:8080/job/hpc-ucx-pr/10070/ for details (Mellanox internal link).

swx-jenkins1 · 2019-04-30T07:56:59Z

Test PASSed.
See http://bgate.mellanox.com/jenkins/job/gh-ucx-pr/7206/ for details.

mellanox-github · 2019-04-30T09:23:53Z

Test FAILed.
See http://hpc-master.lab.mtl.com:8080/job/hpc-ucx-pr/10071/ for details (Mellanox internal link).

hiroyuki-sato · 2019-04-30T09:39:21Z

Hello, @yosefe
Could you tell me more detail MellanoxLab test results?
Could you send me the test output?
Those code passed in my environment and Mellanox

Best regards.

swx-jenkins1 · 2019-04-30T13:59:21Z

Test PASSed.
See http://bgate.mellanox.com/jenkins/job/gh-ucx-pr/7210/ for details.

mellanox-github · 2019-04-30T15:18:05Z

Test FAILed.
See http://hpc-master.lab.mtl.com:8080/job/hpc-ucx-pr/10076/ for details (Mellanox internal link).

yosefe · 2019-05-01T12:56:06Z

@hiroyuki-sato there is a Coverity error:
http://bgate.mellanox.com/jenkins/job/hpc-ucx-pr__published_from_mellanox_lab/10076/label=hpc-test-node,worker=3/artifact/cov_build_10076/output/errors/index.html

src/ucs/sys/event_set.c

src/ucs/sys/event_set.h

yosefe · 2019-05-01T13:06:39Z

@hiroyuki-sato pls also squash all commits (we have too many already: 37) and commit title would be:
"UCS/SYS/TEST: Add event set abstraction"

hiroyuki-sato

@yosefe Thank you for your review. I squashed all commits. Please take a look again.

src/ucs/sys/event_set.c

src/ucs/sys/event_set.h

test/gtest/ucs/test_event_set.cc

swx-jenkins1 · 2019-05-01T15:38:01Z

Test PASSed.
See http://bgate.mellanox.com/jenkins/job/gh-ucx-pr/7220/ for details.

hiroyuki-sato

@yosefe Thank you for the comment. I changed about your comments. Please take a look again when you get a chance.

src/ucs/sys/event_set.c

mellanox-github · 2019-05-01T17:00:33Z

Test FAILed.
See http://hpc-master.lab.mtl.com:8080/job/hpc-ucx-pr/10088/ for details (Mellanox internal link).

swx-jenkins1 · 2019-05-01T17:40:10Z

Test PASSed.
See http://bgate.mellanox.com/jenkins/job/gh-ucx-pr/7222/ for details.

mellanox-github · 2019-05-01T19:37:51Z

Test PASSed.
See http://hpc-master.lab.mtl.com:8080/job/hpc-ucx-pr/10090/ for details (Mellanox internal link).

shamisp · 2019-05-01T20:33:17Z

src/ucs/sys/event_set.c

+#  include "config.h"
+#endif
+
+#define UCS_EVENT_EPOLL_MAX_EVENTS 16


@yosefe Why it is 16 ?

@shamisp JFYI This value just copied from https://github.com/openucx/ucx/blob/master/src/ucs/async/thread.c#L20

shamisp · 2019-05-02T13:45:17Z

👍

hiroyuki-sato commented Mar 28, 2019

View reviewed changes

src/ucs/sys/event_set.c Outdated Show resolved Hide resolved

shamisp added Portability WIP-DNM Work in progress / Do not review labels Mar 28, 2019

shamisp requested review from shamisp and yosefe March 28, 2019 20:03

yosefe reviewed Mar 31, 2019

View reviewed changes

hiroyuki-sato commented Apr 9, 2019

View reviewed changes

src/ucs/sys/event_set.c Show resolved Hide resolved

src/ucs/sys/event_set.h Outdated Show resolved Hide resolved

test/gtest/ucs/test_event_set.cc Outdated Show resolved Hide resolved

test/gtest/ucs/test_event_set.cc Show resolved Hide resolved

yosefe requested changes Apr 11, 2019

View reviewed changes

yosefe added the Waiting for Author Response label Apr 14, 2019

hiroyuki-sato mentioned this pull request Apr 21, 2019

Build on macOS (UCS TODO List) hiroyuki-sato/ucx#1

Open

yosefe reviewed May 1, 2019

View reviewed changes

src/ucs/sys/event_set.c Show resolved Hide resolved

src/ucs/sys/event_set.h Outdated Show resolved Hide resolved

hiroyuki-sato force-pushed the event_set_functions branch from 9020dbf to 69fb2d5 Compare May 1, 2019 14:29

hiroyuki-sato commented May 1, 2019

View reviewed changes

src/ucs/sys/event_set.c Show resolved Hide resolved

src/ucs/sys/event_set.h Show resolved Hide resolved

test/gtest/ucs/test_event_set.cc Outdated Show resolved Hide resolved

UCS/SYS/TEST: Add event set abstraction

6746858

hiroyuki-sato force-pushed the event_set_functions branch from 69fb2d5 to 6746858 Compare May 1, 2019 16:28

hiroyuki-sato commented May 1, 2019

View reviewed changes

src/ucs/sys/event_set.c Show resolved Hide resolved

yosefe approved these changes May 1, 2019

View reviewed changes

yosefe changed the title ~~[WIP] UCS/API event_set~~ UCS/API event_set May 1, 2019

yosefe added Ready for Review and removed Waiting for Author Response labels May 1, 2019

shamisp reviewed May 1, 2019

View reviewed changes

shamisp merged commit 1ad5b17 into openucx:master May 2, 2019

hiroyuki-sato mentioned this pull request May 21, 2019

ucs/async/thread.c use event_set API #3610

Merged

hiroyuki-sato deleted the event_set_functions branch June 30, 2020 13:35

UCS/API event_set #3426

UCS/API event_set #3426

Conversation

hiroyuki-sato commented Mar 28, 2019

What

Why ?

How ?

swx-jenkins1 commented Mar 28, 2019

shamisp commented Mar 28, 2019

swx-jenkins1 commented Mar 28, 2019

mellanox-github commented Mar 28, 2019

yosefe commented Mar 31, 2019

hiroyuki-sato commented Apr 1, 2019

yosefe commented Apr 1, 2019

swx-jenkins1 commented Apr 1, 2019

hiroyuki-sato commented Apr 1, 2019

mellanox-github commented Apr 1, 2019

swx-jenkins1 commented Apr 1, 2019

mellanox-github commented Apr 1, 2019

hiroyuki-sato left a comment

Choose a reason for hiding this comment

swx-jenkins1 commented Apr 9, 2019

mellanox-github commented Apr 9, 2019

hiroyuki-sato commented Apr 12, 2019

hiroyuki-sato commented Apr 20, 2019

swx-jenkins1 commented Apr 20, 2019

swx-jenkins1 commented Apr 20, 2019

mellanox-github commented Apr 20, 2019

mellanox-github commented Apr 20, 2019

mellanox-github commented Apr 29, 2019

mellanox-github commented Apr 29, 2019

swx-jenkins1 commented Apr 30, 2019

mellanox-github commented Apr 30, 2019

swx-jenkins1 commented Apr 30, 2019

mellanox-github commented Apr 30, 2019

hiroyuki-sato commented Apr 30, 2019

swx-jenkins1 commented Apr 30, 2019

mellanox-github commented Apr 30, 2019

yosefe commented May 1, 2019

yosefe commented May 1, 2019 • edited Loading

hiroyuki-sato left a comment

Choose a reason for hiding this comment

swx-jenkins1 commented May 1, 2019

hiroyuki-sato left a comment

Choose a reason for hiding this comment

mellanox-github commented May 1, 2019

swx-jenkins1 commented May 1, 2019

mellanox-github commented May 1, 2019

shamisp May 1, 2019

Choose a reason for hiding this comment

hiroyuki-sato May 2, 2019

Choose a reason for hiding this comment

shamisp commented May 2, 2019

yosefe commented May 1, 2019 •

edited

Loading