Cool VL Viewer forum
http://sldev.free.fr/forum/

IO Completion Port based LLLFSThread for windows & Linux
http://sldev.free.fr/forum/viewtopic.php?f=10&t=2222
Page 2 of 5

Author:  Henri Beauchamp [ 2021-09-30 21:39:13 ]
Post subject:  Re: IO Completion Port based LLLFSThread for windows & Linux

kathrine wrote:
the mOffset stuff is kind of weird for READ...

The old code does an llassert(mOffsett >= 0) and then goes on to handle the case that mOffset <0 for seeking from the end. So that should basically never happen.
Not really, because llassert() is only actually executed for DEBUG builds... :-P

But yes, that llassert() shall be removed... or replaced with llassert_always() and the mOffset < 0 stuff removed.

Author:  kathrine [ 2021-09-30 22:26:13 ]
Post subject:  Re: IO Completion Port based LLLFSThread for windows & Linux

I think i found the sound issue.

The problem is the fetching of the completions, it seems that for sounds it never fires.

If i move the Completion part into a LLLFSThread::runCondition() method instead of the processNextRequest() it fires and sounds start to play again.

Might be related how the audio part handles its completions.

Author:  Henri Beauchamp [ 2021-09-30 22:41:34 ]
Post subject:  Re: IO Completion Port based LLLFSThread for windows & Linux

kathrine wrote:
I think i found the sound issue.

The problem is the fetching of the completions, it seems that for sounds it never fires.

If i move the Completion part into a LLLFSThread::runCondition() method instead of the processNextRequest() it fires and sounds start to play again.

Might be related how the audio part handles its completions.

Just a quick update with some more (superficial) testing I made: sound does not seem to be the only affected type of assets. I also get a lot of animation duplicate requests warnings in the log...

Author:  kathrine [ 2021-09-30 23:04:10 ]
Post subject:  Re: IO Completion Port based LLLFSThread for windows & Linux

For sounds it is the following mix of problems (for Windows currently, but probably Linux is the same).

1) mBytesRead is reset to 0 in ReadFile/WriteFile before handing off the OVERLAPPED Calls
2) The llaudiodecodemgr uses mBytesRead as a marker for completion, as it sets it to -1 first and then sets it to the correct value in completed().
3) So when the async I/IO starts, the file handle is in use, but then the audio part checks for size=0, logs an error and tries to fix it by deleting the file (which fails).

That spams the log file and can be fixed by using NULL for the result param, so it does not get zeroed.
DWORD status = ReadFile(mFile, mBuffer, mBytes, NULL, &mOverlapped);

The second (and main) issue seems to be that the GetQueuedCompletionStatus(mIOCPPort, &bytesProcessed, &key, &result, 0); or equivalent io_uring_peek_cqe() call isn't reached, as the LFS thread goes to sleep. If i move the completions to the runCondition() method instead, the completions get processed just fine.

Author:  kathrine [ 2021-10-01 16:23:46 ]
Post subject:  Re: IO Completion Port based LLLFSThread for windows & Linux

I did a few changes on the Windows side of the patch.

This restores sound for me and i noticed no strange log messages anymore with it present. Changes are mostly the same as describe, i moved the Completion into the "threadedUpdate" method of the QueuedThread, not sure if thats a good place or if it should be moved to somewhere else alltogether.
With some more fixes, it works with proper sound on Linux as well, at least for my quick tests.

I didn't see the other messages you mentioned about animations or other assets, hopefully those are also now working.

Attachments:
io_uring_v5.patch.gz [19.83 KiB]
Downloaded 100 times

Author:  Henri Beauchamp [ 2021-10-01 19:42:17 ]
Post subject:  Re: IO Completion Port based LLLFSThread for windows & Linux

kathrine wrote:
I did a few changes on the Windows side of the patch.

This restores sound for me and i noticed no strange log messages anymore with it present. Changes are mostly the same as describe, i moved the Completion into the "threadedUpdate" method of the QueuedThread, not sure if thats a good place or if it should be moved to somewhere else alltogether.
With some more fixes, it works with proper sound on Linux as well, at least for my quick tests.

A couple of things seem off to me in this new patch:
  • There's obviously a missing "sRingInitialized = true;" for Windows, just after the "mIOCPPort = CreateIoCompletionPort(...);" line.
  • You changed mBytesRead(0) to mBytesRead(-1) in the request constructor but this might break the non-io_uring code (used when sUseIoUring is false). I suggest instead using mBytesRead(sUseIoUring ? -1 : 0)

Also, the LLLFSThread::processNextRequest() method can be removed entirely and the matching processNextRequest() method in LLQueueThread can be made non-virtual again.

Author:  Henri Beauchamp [ 2021-10-01 20:22:07 ]
Post subject:  Re: IO Completion Port based LLLFSThread for windows & Linux

Well, I have been using this slightly modified patch (with the few changes described above), but it still fails to play (non-cached) sounds for me under Linux (not yet tested under Windows here):

Attachments:
io_uring-v6.patch.gz [8.33 KiB]
Downloaded 95 times

Author:  kathrine [ 2021-10-01 20:49:02 ]
Post subject:  Re: IO Completion Port based LLLFSThread for windows & Linux

I looked at your updated patch.

The issue with sound comes from setting mBytesRead = 0 too early (while the async read/write is still in flight).

So adding the "mBytesRead = mBytes; // Success" back, breaks sound again.

The problem/reason for this is in llaudio/llaudiodecodemgr.cpp around line 495:
Code:
      mBytesRead = -1;
      mFileHandle = LLLFSThread::sLocal->write(mOutFilename,
                                     &mWAVBuffer[0], 0,
                                     mWAVBuffer.size(),
                                     new WriteResponder(this));
   }

   if (mFileHandle != LLLFSThread::nullHandle())
   {
      if (mBytesRead < 0)
      {
         return false; // not done
      }
      if (mBytesRead == 0)
      {
         llwarns_once << "Unable to write file for " << getUUID() << llendl;
         mValid = false;
         return true; // we've finished
      }
   }

   mDone = true;

#if 0
   LL_DEBUGS("Audio") << "Finished decode for " << getUUID() << LL_ENDL;
#endif

   return true;


As you can see, it uses the -1 as a marker for "not yet done" and 0 or more as marker for "done". So if the lllfsthread codes sets the mBytesRead to anything but -1 before the actual async read/write is completed (for a sound), this breaks down and the error handler for sounds kicks in and loading of sounds fails.

That should work for my patch and i see .dsf files created in the cache area properly. I wonder why it doesn't work for you.

Do the completions get collected ( does io_uring_peek_cqe return 0 and populate a cqe? Your patch does "have_work = io_uring_peek_cqe()" which has the logic reversed ).
I had some issues when the LFS thread was set as idle and slept and completions would not get collected in my first tries.

Edit: Missed the (non-cached) part. Will have to check it works properly with a cleaned cache.

Author:  ZaneZimer [ 2021-10-01 20:52:58 ]
Post subject:  Re: IO Completion Port based LLLFSThread for windows & Linux

I grabbed Henri's patch and applied it. A quick build and test has sounds playing for me in Linux. Is there anything I can do to help test or narrow down? The only 'custom' I have in my builds is using the new curl.

Author:  Henri Beauchamp [ 2021-10-01 20:53:57 ]
Post subject:  Re: IO Completion Port based LLLFSThread for windows & Linux

kathrine wrote:
So adding the "mBytesRead = mBytes; // Success" back, breaks sound again.
I removed that from my latest patch... Might have uploaded an intermediate "v6" patch... Still no joy with that line removed anyway.

Quote:
That should work for my patch and i see .dsf files created in the cache area properly. I wonder why it doesn't work for you.
It does not work for me...

Quote:
Do the completions get collected ( does io_uring_peek_cqe return 0 and populate a cqe? Your patch does "have_work = io_uring_peek_cqe()" which has the logic reversed ).
Again, this was since fixed...
Quote:
I had some issues when the LFS thread was set as idle and slept and completions would not get collected in my first tries.
I can only play *cached* (in the asset cache) sounds here...

Page 2 of 5 All times are UTC
Powered by phpBB® Forum Software © phpBB Group
https://www.phpbb.com/