Make SO_POLL complete asynchronously in IOS_NET SO #8813

nbouteme · 2020-05-16T23:18:09Z

This is to make the behavior more consistent with the behavior on console.

Currently, SO_POLL blocks the PPC until poll() completes, so this prevents PPC threads on DolphinOS and libogc's LWP from running while waiting for the call to complete, while on a real console, IOS lets the PPC run, as with most other calls, and notifies the PPC through the usual mechanism.

I don't know if how I implemented it is the preferred way in regard to Dolphin's internals: Basically each SO_POLL operation gets queued into a poll table with the associated request, and when an event happens on a socket that is referenced in a poll table in WiiSockMan::Update, an IPC reply is generated for the request.

sepalani

This PR sounds fine to me but there are things that should be addressed.

You should probably squashed your 2 commits together.
Please, have a look at Contributing.md.
Then, check with clang-format that the formatting is correct.

Source/Core/Core/IOS/Network/IP/Top.cpp

sepalani · 2020-05-17T14:09:50Z

Source/Core/Core/IOS/Network/Socket.cpp

+      const int write_event = (POLLOUT | POLLWRBAND);
+      const int read_event = (POLLIN | POLLRDNORM | POLLRDBAND | POLLPRI);
+      ufds.push_back({sock.fd, read_event | write_event, 0});
+      nfds = std::max(nfds, sock.wii_fd + 1);


Good catch, the wii_fd should be used here.

Source/Core/Core/IOS/Network/Socket.cpp

delroth · 2020-05-17T14:45:59Z

This will leave an unanswered/hanging IPC when saving/loading states.

nbouteme · 2020-05-17T15:51:20Z

Thanks for the comments. I'll fix these ASAP.

This will leave an unanswered/hanging IPC when saving/loading states.

This means I should add a DoState method to WiiSockMan, and answer IPC calls with errors when loading from a save state, right?

EDIT: Instead of making SO_POLL return an error, I made it return the events POLLHUP | POLLERR which made more sense.

delroth · 2020-05-17T15:53:49Z

I think that would make the most sense, yes.

Source/Core/Core/IOS/Network/Socket.cpp

Source/Core/Core/IOS/Network/IP/Top.h

Source/Core/Core/IOS/Network/Socket.h

Source/Core/Core/IOS/Network/Socket.cpp

Source/Core/Core/IOS/Network/IP/Top.cpp

sepalani · 2020-05-19T08:53:31Z

Source/Core/Core/IOS/Network/Socket.cpp

  auto socket_iter = WiiSockets.begin();
  auto end_socks = WiiSockets.end();
+  ufds.clear();


Can't the ufds variable be local to this function rather than a class member? It doesn't seem to be used elsewhere.

Is it really appropriate to allocate and deallocate a vector in an update function? clear() preserves the allocated capacity.

Source/Core/Core/IOS/Network/Socket.cpp

sepalani · 2020-05-19T09:53:04Z

Source/Core/Core/IOS/Network/Socket.cpp

+    n++;
+  }
+
+  auto mid = std::partition(pending_polls.begin(), pending_polls.end(), [&](auto& pcmd) {


Can't a similar logic like WiiSocket::Update be used here or is there a performance issue?

auto it = pending_polls.begin(); while (it != pending_polls.end()) { // SOME CODE if (shall_return || has_timeout || whatever_exit_code) { // Write IPC Reply it = pending_polls.erase(it); } else { ++it; }

This logic might be able to get rid of the finished_polls and failed_polls member variables.

I just replaced that with an equivalent erase-remove idiom.
It still allowed me to remove these two member variables. The only performance benefit to doing it this way instead of iterating is to do only one erase operation, which doesn't really matter in typical cases (IOS doesn't allow polling more than a couple tens of file descriptors). The other benefits in using the stdlib for container manipulations are purely theoretical, but are (maybe) still there. But mostly being less error prone.

My only concern about doing it this way, is that it will call begin()/end()/remove_if()/erase() every time regardless of the pending_polls size. I also think that the whole update poll logic should belong to its own method: "UpdatePollCommands" or something like that.

Most (if not all - can't speak for MSVC) of these calls are inlined, the only external reference (on g++) is a realloc_insert call, probably for the push_back on ufds, which will probably never get called since its size is preallocated before the loop. I'll move all that logic into a separate function, though, it's true that it's too big to leech off another method.

Source/Core/Core/IOS/Network/Socket.cpp

nbouteme · 2020-05-20T19:55:26Z

Hm, this type error on the poll() call didn't happen on the previous windows builds. And MSVC doesn't seem to like the implicit constexpr capture from outside the scope of a lambda, even though the standard allows it. Or maybe I'm reading the error wrong.

JosJuice · 2020-05-20T19:57:43Z

And MSVC doesn't seem to like the implicit constexpr capture from outside the scope of a lambda, even though the standard allows it.

I've had that problem as well (though only when building with CMake, for some reason). I'd suggest just capturing it to make MSVC happy.

BhaaLseN · 2020-05-20T21:25:18Z

Source/Core/Core/IOS/Network/Socket.cpp

+  {
+    int native;
+    int wii;
+  } mapping[] = {


I wonder if this could (or even should) be static (and maybe const), perhaps even constexpr?

constexpr seems to work

nbouteme · 2020-05-20T21:40:05Z

I set error_event as a constexpr static since on MSVC, explicitly capturing a constexpr seem to make the compiler disregard the constexpr qualifier? But this makes GCC 10 on Linux print a warning about the variable being set but unused despite being really used...

And about poll, I had to cast to an u32 since Windows doesn't provide the posix nfds_t type...

lioncash · 2020-05-20T21:48:55Z

Source/Core/Core/IOS/Network/Socket.cpp

+
+void WiiSockMan::UpdatePollCommands()
+{
+  constexpr static int error_event = (POLLHUP | POLLERR);


The static isn't necessary here. Given it's a trivial primitive type, this will be constant folded into expressions anyway (especially given it's constexpr).

Our convention is also to place the lifetime specifiers before mutability ones, just to keep in mind for the future.

I know, but it won't build on Windows without it due to a compiler bug. Other alternative is capturing it explicitly but this implicitly discards the constexpr qualifier

Source/Core/Core/IOS/Network/Socket.cpp

Source/Core/Core/IOS/Network/Socket.h

sepalani

Sorry, I missed a few things here and there.

Source/Core/Core/IOS/Network/IP/Top.cpp

Source/Core/Core/IOS/Network/Socket.cpp

Source/Core/Core/IOS/Network/IP/Top.cpp

Source/Core/Core/IOS/Network/Socket.cpp

Source/Core/Core/IOS/Network/IP/Top.cpp

Source/Core/Core/IOS/Network/Socket.cpp

Source/Core/Core/IOS/Network/IP/Top.cpp

Source/Core/Core/IOS/Network/Socket.cpp

sepalani · 2020-05-21T18:46:44Z

Source/Core/Core/IOS/Network/Socket.cpp

@@ -685,25 +686,23 @@ s32 WiiSockMan::DeleteSocket(s32 s)

 void WiiSockMan::Update()
 {
-  s32 nfds = 0;


You shouldn't call poll if WiiSockets is empty. Otherwise, it spams error logs on Windows. Regardless, you still need to clear previous ufds values and update poll commands.

sepalani · 2020-05-28T09:58:36Z

It seems Windows is still having issues, I'll have look to see what is going on.

sepalani

Could you please squash all your commits into a single one?

sepalani · 2020-05-28T12:56:17Z

Source/Core/Core/IOS/Network/IP/Top.h

@@ -13,6 +13,13 @@
 #include <ws2tcpip.h>
 #endif

+// WSAPoll doesn't support POLLPRI and POLLWRBAND flags
+#ifdef _WIN32
+#define UNSUPPORTED_WSAPOLL POLLPRI | POLLWRBAND


Sorry, that was an oversight of #3316. It was missing parentheses.

Can you please change it to a constexpr constant instead

constexpr int UNSUPPORTED_WSAPOLL = POLLPRI | POLLWRBAND;

sepalani · 2020-05-28T17:13:31Z

This PR introduces a regression in DJ Hero. I'll try to see where it's from.

On master

Network - DemonWare::SetAuthenticationAddress AUTH IP = 185.34.107.68   LSG IP = 185.34.107.72
BeginAccountStage - Success, ENETWORK_MANAGER_ACCOUNT.
InternalAccountUpdate - Success, ENETWORK_MANAGER_LOBBY.
InternalLobbyConnectUpdate - Success, ENETWORK_MANAGER_READY.
**** Lobby connected successfully ****

On this PR

Network - DemonWare::SetAuthenticationAddress AUTH IP = 185.34.107.68   LSG IP = 185.34.107.72
BeginAccountStage - Success, ENETWORK_MANAGER_ACCOUNT.
CNetworkAuthenticateAccountState - Authorization server failed in an unexpected manner.
Failed to connect to lobby service.
CNetworkManager::Fail_InternalAccount()
****** Global Network Error: 12.

nbouteme · 2020-05-28T18:14:25Z

Does it require real nand certificates to reproduce?
What are the required steps?

sepalani · 2020-05-28T18:25:59Z

You'll need a real NAND and find the logging function (OSReport, ___blank, etc.) which should be found using "Generate symbols from > Signature database" if you're lucky. After, that these messages should appear on the log console if OSReport logs are enabled.

At first glance, it seems to be a timing-issue since I can reproduce the issue on master if I put a breakpoint on the SOConnect function and wait about 3~5 seconds before resuming.

On master, the game says that the DJ Hero servers are unavailable.
On your PR, it says that the game servers are unavailable.

nbouteme · 2020-05-28T18:31:39Z

Unfortunately I don't have access to a real nand at the moment.
Does IOCTL_SO_POLL appear in the log at debug level?

sepalani · 2020-05-28T18:36:31Z

No, it doesn't. I just hope, that's not yet another issue with WSAPoll implementation, otherwise, it will be a blocker for Windows.

nbouteme · 2020-05-28T18:37:38Z

I managed to reproduce the in-game error message without certificates under Linux. OSReport is detected without generating symbols from the signature database, but I don't see these messages.

sepalani · 2020-05-28T18:43:32Z

There are multiple logging functions, so you might need to generate symbols or do a bit of reverse-engineering to find this one.

nbouteme · 2020-05-28T18:49:37Z

It actually does multiple SO_POLL calls, see attached log.
dolphin.log

nbouteme · 2020-05-28T20:17:25Z

Ok, I've figured it out.

It seems like when you do a poll with both POLLOUT and POLLWRNORM, the OS won't set POLLWRNORM even though they are equivalent. I think the most obvious solution is to re poll with a 0 timeout with the correct events.

Well, at least doing so lets me get the "DJ Hero servers are unavailable" error on Linux. But I don't know if Windows will behave the same, and how nand certs may influence the login process.

sepalani · 2020-05-29T05:37:38Z

IMHO, that is not the best way to address this issue since:

Some FDs might become invalid (after a close for instance) on the second poll call and it might fail.
Some poll implementations might clear or check revents before assigning new values to it.
WSAPoll seems to shadow some error codes.

Unfortunately, the best way I can see to address the issue would be to revert back to select on Update and call poll for each pending_polls with the appropriate events set on UpdatePollCommands.

I expect small performance impacts on games using poll but at the very list it won't be blocking anymore.

nbouteme · 2020-05-29T13:04:44Z

Shouldn't most of these issues be solved simply by lazily pruning closed sockets from each pending_commands ? I don't see what reverting back to select will accomplish. The remaining issue is that poll may get called on pending_commands after a savestate load, so I probably should just clear the revents fields before ending the update, and not poll commands with non-zero revents.

sepalani · 2020-05-29T13:45:43Z

Reverting back to select will fix Update regressions on Windows since WSAPoll introduces issues due to its poll implementation being different.

On top of that you pointed out an issue forcing Dolphin to use poll twice while monitoring all events. In other word, I'm pretty sure that relying on such behaviours will cause other issues since not all poll implementations behave the same way.

In sum, it's safer and less error-prone (and probably more accurate) to store poll events in the PollCommand structure and call each requested poll in an async manner in UpdatePollCommands.

sepalani

I'll write some hardware tests to confirm some of these.

sepalani · 2020-05-30T08:26:27Z

Source/Core/Core/IOS/Network/Socket.cpp

+            auto& pfds = pcmd.wii_fds;
+            // Happens only on savestate load
+            int count;
+            if (pcmd.wii_fds[0].revents & error_event)


I think you shouldn't bother with the pollfd structures when a savestate is loaded. Just EnqueueIPCReply with -SO_ENETRESET.

Wouldn't games get confused from receiving SO_ENETRESET after they initialized the SO module? I think the worst they are expected to handle is every socket hanging up.

Fair point. Technically, I assume that could happens if you loose your Wi-Fi/Ethernet connection. Anyway, feel free to disregard this comment as your approach is valid.

Source/Core/Core/IOS/Network/Socket.cpp

Source/Core/Core/IOS/Network/IP/Top.cpp

sepalani · 2020-05-31T16:01:12Z

Here is an hardware test:
https://gist.github.com/sepalani/21db2c0d703f0350590cab76062360b5

Regarding the results:

The valid nfds are 1 <= nfds <= 24.
Negative and invalid fds are "ignored" but ...
revents is always clear/set to zero.
The pollfd order doesn't seem to matter since they are all set properly if an event occurs.

Run the commented tests at your own risk. The result is the one expected, poll will work when the fds become valid BUT it seems to break most network features afterward. Basically, you'll have IOS_IOctl(v) returning successful return values, however, you can't interact with the socket properly. Listening on a port appears to work (based on the return values from IOS) but doesn't when you try to connect to it.

sepalani

Thanks for your patience. After these changes based on hardware tests, I believe this PR should be ready.

Source/Core/Core/IOS/Network/Socket.cpp

Source/Core/Core/IOS/Network/IP/Top.cpp

sepalani · 2020-05-31T16:39:47Z

Source/Core/Core/IOS/Network/Socket.cpp

+            auto& pfds = pcmd.wii_fds;
+            // Happens only on savestate load
+            int count;
+            if (pcmd.wii_fds[0].revents & error_event)


Fair point. Technically, I assume that could happens if you loose your Wi-Fi/Ethernet connection. Anyway, feel free to disregard this comment as your approach is valid.

Source/Core/Core/IOS/Network/Socket.cpp

sepalani

Could you please squash your commits into a single one.

sepalani · 2020-06-01T05:05:41Z

Source/Core/Core/IOS/Network/IP/Top.cpp

+  // Negative timeout indicates wait forever
+  const s64 timeout = static_cast<s64>(Memory::Read_U64(request.buffer_in));
+
+  const unsigned nfds = request.buffer_out_size / 0xc;


const u32 nfds

sepalani · 2020-06-01T05:06:22Z

Source/Core/Core/IOS/Network/IP/Top.cpp


  std::vector<pollfd_t> ufds(nfds);

-  for (int i = 0; i < nfds; ++i)
+  for (unsigned i = 0; i < nfds; ++i)


sepalani · 2020-06-01T05:15:09Z

Source/Core/Core/IOS/Network/Socket.cpp

+            }
+            else
+            {
+              // Update FDs to a negative FD in case they were closed:


This comment needs to be changed to something more fitting like:

// Make the behavior of poll consistent across platforms by not passing: // - Set with invalid fds, revents is set to 0 (Linux) or POLLNVAL (Windows) // - Set without a valid socket, raises an error on Windows

sepalani · 2020-06-01T05:16:59Z

Source/Core/Core/IOS/Network/Socket.cpp

+                return GetHostSocket(Memory::Read_U32(pcmd.buffer_out + 0xc * i)) > 0;
+              });
+              // move all the valid pollfds to the front of the vector
+              for (auto i = 0; i < mid - original_order.begin(); ++i)


Can't mid - original_order.begin() be replaced with n_valid?

sepalani · 2020-06-01T05:17:39Z

Source/Core/Core/IOS/Network/Socket.cpp

+              auto mid = std::partition(original_order.begin(), original_order.end(), [&](auto i) {
+                return GetHostSocket(Memory::Read_U32(pcmd.buffer_out + 0xc * i)) > 0;
+              });
+              // move all the valid pollfds to the front of the vector


sepalani · 2020-06-01T05:18:51Z

Source/Core/Core/IOS/Network/Socket.cpp

+              if (count < 0)
+                count = GetNetErrorCode(count, "UpdatePollCommands", false);
+
+              // move everything back to where they were


sepalani · 2020-06-01T05:24:10Z

Source/Core/Core/IOS/Network/Socket.cpp

+          [this](auto& pcmd) {
+            const auto request = Request(pcmd.request_addr);
+            auto& pfds = pcmd.wii_fds;
+            int count = 0;


This should probably be renamed ret as it might not return the count but a network error code.

sepalani · 2020-06-01T06:00:26Z

Could you please change/remove the commit description:

This also replaces the select() call in WiiSockMan::Update by a poll call ...

Do you have any games or homebrew that use SOPoll so we can test this PR against?

nbouteme · 2020-06-01T06:08:20Z

Well besides DJ Hero and your test suite, no, I don't. I'll try to look around if I find some larger homebrew.

I'd suspect online games with a large active modding community like MKWii to be very likely to use it.

nbouteme · 2020-06-01T06:32:14Z

On the homebrew side:
WiiRadio
WiiFlow Lite
Quake GX

sepalani

The commit description is still not amended.

I haven't found issues with this PR while doing some testing. I haven't tried multiplayer games yet.

sepalani · 2020-06-01T14:20:48Z

Source/Core/Core/IOS/Network/Socket.cpp

+            int ret = 0;
+
+            // Happens only on savestate load
+            if (pcmd.wii_fds[0].revents & error_event)


if (pfds[0].revents & error_event)

sepalani · 2020-06-01T14:21:26Z

Source/Core/Core/IOS/Network/Socket.cpp

+            // Happens only on savestate load
+            if (pcmd.wii_fds[0].revents & error_event)
+            {
+              ret = static_cast<int>(pcmd.wii_fds.size());


ret = static_cast<int>(pfds.size());

nbouteme · 2020-06-01T20:44:29Z

I'm sorry, I'm not much proficient with git. Isn't amending just git commit --amend and force push, or am I forgetting something here? Or is the description still not what you wanted?

sepalani · 2020-06-02T04:55:54Z

@nbouteme No worries. Feel free to ask or go over IRC if you're having troubles with git.

In fact, you're amending your commit but not the commit message/description.
https://help.github.com/en/github/committing-changes-to-your-project/changing-a-commit-message

During the amend process, you need to change/remove the commit description (i.e. the sentences under the commit message) with you text editor. Alternatively, you can do it directly from command-line:

git commit --amend -m "IOS/Network: Make poll complete asynchronously"

If you want to change the commit description you'll need to pass an extra -m flag:

git commit --amend -m "commit message" -m "commit description"

You can check your commit history using git log and see if amending worked. After that, you can force push.

sepalani

Otherwise, the logic LGTM. I'm going to test it against some multiplayer games later today to confirm it doesn't introduce regressions.

sepalani · 2020-06-07T06:17:48Z

Source/Core/Core/IOS/Network/IP/Top.cpp

+  const s64 timeout = static_cast<s64>(Memory::Read_U64(request.buffer_in));
+
+  const u32 nfds = request.buffer_out_size / 0xc;
+  if (nfds <= 0 || nfds > WII_SOCKET_FD_MAX)


if (nfds == 0 || nfds > WII_SOCKET_FD_MAX)

sepalani · 2020-06-07T06:18:56Z

Source/Core/Core/IOS/Network/Socket.cpp

+              std::iota(original_order.begin(), original_order.end(), 0);
+              // Select indices with valid fds
+              auto mid = std::partition(original_order.begin(), original_order.end(), [&](auto i) {
+                return GetHostSocket(Memory::Read_U32(pcmd.buffer_out + 0xc * i)) > 0;


AFAIK, 0 is a valid socket fd, so it should be >= 0.

sepalani · 2020-06-07T06:24:18Z

Source/Core/Core/IOS/Network/IP/Top.cpp

+  const u32 nfds = request.buffer_out_size / 0xc;
+  if (nfds <= 0 || nfds > WII_SOCKET_FD_MAX)
+  {
+    ERROR_LOG(IOS_NET, "IOCTL_SO_POLL failed: Invalid array size %d, ret= %d", nfds, -SO_EINVAL);


ret=%d (without the space)

sepalani

LGTM. Tested on Windows 10 with Super Smash Bros. Brawl, Mario Strikers Charged Football, DJHero. I didn't find regression with them. I tested SSBB & MSC multiplayer modes using altwfc servers.

sepalani · 2020-06-24T10:44:08Z

@nbouteme Could you please rebase your PR to the latest master and update the STATE_VERSION?

JMC47 · 2020-06-26T08:29:56Z

Is there any particular game or case that this helps so I could properly make a demonstration of how it affects things? If it's all internally changing things, that's fine too, I just wanted to know if I could make some media for the Progress Report.

Thanks!

nbouteme · 2020-06-26T09:06:52Z

Thanks for merging, and also thanks a lot to sepalani who helped a lot to make this a proper PR.

Unfortunately I don't think this change would be really visible to end users : it theoretically eliminates stutters, or possible deadlocks causing games to hang, when a game uses SO_POLL with a non-zero timeout, but I'm don't really know whether wii games do that often. I don't think they do, or else the stutter would have probably broken some games and this would have been fixed way earlier. On the other hand, it may also help with some homebrews but I don't think this is most users use cases. My use case was a mod to add some online functionnality to a game that lacked it, but using poll with a timeout of 0 didn't sit right with me... It's not as efficient as I'd like it to be because in the end, poll and select still need to be called with a 0 timeout, since IOS HLE runs on the emulation thread. This still allows people to write mods and have a behavior that's more in line with what would happen on a real wii. A thread waiting on a SO_POLL call to return will remain dormant instead of being immediately woken up then immediately sleep again, so it reduces CPU usage on PPC.

If IOS HLE ran on a separate thread, then the socket polling mechanism wouldn't be executed as often and would reduce CPU usage in HLE code by a bit, but this poses a lot of harder challenges. But if/when this will eventually be done, then anything that uses non-zero timeouts on SO_POLL will use a bit less CPU. So yeah, it's not that big of a deal in the end, it's more like a convenience for mod developers.

JMC47 · 2020-06-26T09:25:05Z

Yeah, this mixed with other async PRs seem to have eliminated some of the stuttering when connecting to servers online.

If there's no great demonstration yet, once more of it is done I'll definitely be able to show off a bigger change. Mario Kart Wii in particular used to be a mess of 3 - 4 second stutters while connecting, and it's slowly been getting better.

sepalani suggested changes May 17, 2020

View reviewed changes

BhaaLseN reviewed May 17, 2020

View reviewed changes

Source/Core/Core/IOS/Network/Socket.cpp Outdated Show resolved Hide resolved

Source/Core/Core/IOS/Network/IP/Top.h Show resolved Hide resolved

nbouteme requested review from BhaaLseN and sepalani May 17, 2020 22:27

BhaaLseN reviewed May 18, 2020

View reviewed changes

Source/Core/Core/IOS/Network/Socket.h Outdated Show resolved Hide resolved

Source/Core/Core/IOS/Network/Socket.cpp Outdated Show resolved Hide resolved

sepalani suggested changes May 19, 2020

View reviewed changes

nbouteme requested review from sepalani and BhaaLseN May 20, 2020 18:26

sepalani suggested changes May 20, 2020

View reviewed changes

Source/Core/Core/IOS/Network/Socket.cpp Outdated Show resolved Hide resolved

Source/Core/Core/IOS/Network/Socket.cpp Outdated Show resolved Hide resolved

Source/Core/Core/IOS/Network/Socket.cpp Outdated Show resolved Hide resolved

BhaaLseN reviewed May 20, 2020

View reviewed changes

lioncash reviewed May 20, 2020

View reviewed changes

sepalani suggested changes May 21, 2020

View reviewed changes

Source/Core/Core/IOS/Network/IP/Top.cpp Outdated Show resolved Hide resolved

Source/Core/Core/IOS/Network/Socket.cpp Outdated Show resolved Hide resolved

sepalani suggested changes May 21, 2020

View reviewed changes

sepalani suggested changes May 28, 2020

View reviewed changes

sepalani suggested changes May 30, 2020

View reviewed changes

sepalani suggested changes May 31, 2020

View reviewed changes

sepalani suggested changes Jun 1, 2020

View reviewed changes

sepalani suggested changes Jun 7, 2020

View reviewed changes

sepalani approved these changes Jun 9, 2020

View reviewed changes

nbouteme requested a review from BhaaLseN June 10, 2020 05:15

nbouteme requested a review from lioncash June 19, 2020 19:04

Make SO_POLL complete asynchronously

b68ab56

JMC47 merged commit 502ab78 into dolphin-emu:master Jun 25, 2020

Make SO_POLL complete asynchronously in IOS_NET SO #8813

Make SO_POLL complete asynchronously in IOS_NET SO #8813

Conversation

nbouteme commented May 16, 2020 • edited

sepalani left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

delroth commented May 17, 2020

nbouteme commented May 17, 2020 • edited

delroth commented May 17, 2020

Choose a reason for hiding this comment

nbouteme May 19, 2020 • edited

Choose a reason for hiding this comment

sepalani May 19, 2020 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nbouteme May 19, 2020 • edited

Choose a reason for hiding this comment

nbouteme commented May 20, 2020

JosJuice commented May 20, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nbouteme commented May 20, 2020 • edited

Choose a reason for hiding this comment

nbouteme May 20, 2020 • edited

Choose a reason for hiding this comment

sepalani left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sepalani commented May 28, 2020

sepalani left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sepalani commented May 28, 2020

On master

On this PR

nbouteme commented May 28, 2020

sepalani commented May 28, 2020

nbouteme commented May 28, 2020

sepalani commented May 28, 2020

nbouteme commented May 28, 2020 • edited

sepalani commented May 28, 2020

nbouteme commented May 28, 2020 • edited

nbouteme commented May 28, 2020 • edited

sepalani commented May 29, 2020 • edited

nbouteme commented May 29, 2020

sepalani commented May 29, 2020

sepalani left a 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

sepalani commented May 31, 2020

sepalani left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sepalani left a comment • edited

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

sepalani commented Jun 1, 2020

nbouteme commented Jun 1, 2020 • edited

nbouteme commented Jun 1, 2020

sepalani left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nbouteme commented Jun 1, 2020

sepalani commented Jun 2, 2020

sepalani left a 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

nbouteme commented May 16, 2020 •

edited

nbouteme commented May 17, 2020 •

edited

nbouteme May 19, 2020 •

edited

sepalani May 19, 2020 •

edited

nbouteme May 19, 2020 •

edited

nbouteme commented May 20, 2020 •

edited

nbouteme May 20, 2020 •

edited

nbouteme commented May 28, 2020 •

edited

nbouteme commented May 28, 2020 •

edited

nbouteme commented May 28, 2020 •

edited

sepalani commented May 29, 2020 •

edited

sepalani left a comment •

edited

nbouteme commented Jun 1, 2020 •

edited