0002655: Patch to implement progressive texture sending by jhurliman

(0007236)
jhurliman (manager)
2008-11-19 10:45

Several issues cropped up during testing. It appears that OpenJPEG by itself is not thread-safe, so decoding lots of textures at the same time without locking is bound for failure. The idea of JPEG2000 decoding (expensive) every time someone requests a texture is also a very bad move for performance, and ideally this texture metadata should be cached alongside images. The final issue that is a real killer is that it doesn't seem possible to trap segfaults in unmanaged code from Mono, only .NET. Because of this I'm investigating http://www.codeplex.com/csj2k/ [^] to see if the JPEG2000 implementation is complete enough to handle SL-style textures.

(0007303)
jhurliman (manager)
2008-11-24 15:15

I attached openjpeg-dotnet-20081124.zip which includes a recompiled openjpeg-dotnet.dll (I was having problems with the previous compile for some reason) and the latest OpenMetaverse.dll which includes OpenJPEG locking and better error output for failed decodes.

(0007304)
cfk (administrator)
2008-11-24 19:21

I would feel more comfortable if Teravus or Justin took this patch to commit as I dont want to fubar svn tonight.

(0007315)
justincc (manager)
2008-11-25 08:07

Unfortunately we may have to move now to asking people to try this one experimentally before putting it in. I'll try and remember to ask if people can do this at the osgrid meeting today.

(0007606)
HomerHorwitz (manager)
2008-12-06 12:48

Tried to apply it:
- Doesn't apply cleanly to r7631.
- Still contains the sandbox AppDomain code in XEngine

After fixing that (hopefully) and applying it to r7631, textures load without problem here, as long as the textures are without problem.
If I trash an texture-asset entry and try to load the texture, OpenSim crashes.

(0007608)
jhurliman (manager)
2008-12-06 14:33

I need to look into catching the segfaults from C and returning an error code since Mono can't handle crashes in unmanaged code.

(0007995)
jhurliman (manager)
2008-12-16 12:21

<jhurliman> Fly-Man-, the patch works fine as-is right now, unless you feed it bad texture data on mono
<jhurliman> then it crashes, and after talking with the mono team i guess there is no way around that except creating a separate unmanaged process that does texture decoding

(0008024)
Fly-Man- (developer)
2008-12-17 13:48

Patch failed:

patching file prebuild.xml

Reversed (or previously applied) patch detected! Assume -R? [n] Y

Hunk 0000001 succeeded at 409 (offset -52 lines).
Hunk 0000002 succeeded at 627 (offset -79 lines).
Hunk 0000003 succeeded at 660 (offset -79 lines).
Hunk 0000004 succeeded at 940 (offset 108 lines).
Hunk 0000005 succeeded at 1195 (offset 58 lines).
Hunk 0000006 succeeded at 1854 (offset -88 lines).
Hunk 0000007 succeeded at 1895 (offset -88 lines).
Hunk 0000008 FAILED at 2029.
1 out of 8 hunks FAILED -- saving rejects to file prebuild.xml.rej

patching file OpenSim/Region/Environment/Scenes/Scene.Inventory.cs

patching file OpenSim/Region/Environment/Modules/Agent/TextureSender/TextureSender.cs

patching file OpenSim/Region/Environment/Modules/Agent/TextureDownload/TextureDownloadModule.cs

Hunk 0000001 succeeded at 198 (offset 22 lines).

patching file OpenSim/Region/Environment/Modules/Agent/TextureDownload/TextureNotFoundSender.cs

patching file OpenSim/Region/Environment/Modules/Agent/TextureDownload/UserTextureDownloadService.cs

patching file OpenSim/Region/Environment/Modules/Avatar/Friends/FriendsModule.cs
Hunk 0000001 succeeded at 30 (offset 1 line).

patching file OpenSim/Region/ScriptEngine/XEngine/XEngine.cs

patching file OpenSim/Framework/Communications/Capabilities/LLSD.cs
Reversed (or previously applied) patch detected! Assume -R? [n] Y

(0008124)
HomerHorwitz (manager)
2008-12-19 12:06

Handling the segfaults via whatever method possible might be a workaround for our problem, but in my opinion openjpeg mustn't crash on bad data. So, the real fix can only be done on openjpeg.

(0008126)
justincc (manager)
2008-12-19 12:36

Also, sdague brought up the very good point that if a texture can cause a segfault, then an artificially constructed texture could get a shell.

(0008629)
HomerHorwitz (manager)
2009-01-06 15:12

I looked into the crash and found the reason for it (for this particular crash):
The dotnet-wrapper contains some bugs while checking for successful image-decoding. The attached patch (check_image.patch) fixes this bug; after that I didn't manage to crash the region server with the progressive texture patch anymore.

(0008632)
cfk (administrator)
2009-01-06 18:07

Well, are we ready to move forward and apply this patch? Does JHurliman need to do anything else?

(0008633)
jhurliman (manager)
2009-01-06 18:18

We need to push out new openjpeg-dotnet binaries for all of the platforms. Thank you for the patch HomerHorwitz, I'll start compiling this evening or tomorrow.

(0008678)
jhurliman (manager)
2009-01-08 15:21

New source and binaries are in the libomv trunk. I've also attached the binaries to this issue in a zip file for convenience. Drop these in OpenSim's trunk/bin/ and the progressive texture patch should be ready to go.

(0008685)
svn (developer)
2009-01-09 17:46

dahlia committed related code in r8001

Thanks jhurliman for a patch that implements progressive texture downloading - Mantis 0002655

see more at - http://opensimulator.org/cgi-bin/viewvc.cgi?view=rev&revision=8001 [^]

(0008686)
dahlia (manager)
2009-01-09 17:51

applied to r8001. Interested parties please test asap.

(0008690)
dahlia (manager)
2009-01-10 01:22

reverted in r8004. Several issues appeared after commit that did not show up in initial pre-commit testing:
1. Script engines fail when running on Windows platform (see mantis 0002976)
* commenting out the sandbox code in lines 552-558 appeared to resolve this
2. Texture downloads freeze avatar movement and can cause many console errors when logging into a texture rich region after clearing cache

(0008707)
Teravus (administrator)
2009-01-11 12:36
edited on: 2009-01-11 12:49

This is probably a long held lock/possibly a deadlock caused by blockingqueue interaction with the texture send lock. For this to be effective, the decoding needs to be done outside a lock.

Thread track:
UDP Packet request texture--->
Texture Request thread requests texture from assets-->
Texture comes back and locks while openjpeg decodes--->
UDP Packet request texture--->
Texture is in cache and the first UDP packet has locked the object while decoding--->
UDP processing client thread waits....
UDP processing thread does not respond to pings
Client logs off.

(0008912)
Teravus (administrator)
2009-01-21 03:25

Need to separate the decode thread from the client thread and cache the results, but this is in, and it's good!

Issue History
Date Modified	Username	Field	Change
2008-11-18 22:40	melanie	New Issue
2008-11-18 22:40	melanie	File Added: progressive_textures.patch
2008-11-18 22:40	melanie	SVN Revision	=> 0
2008-11-18 22:40	melanie	Run Mode	=> Standalone (1 Region)
2008-11-18 22:40	melanie	Physics Engine	=> ODE
2008-11-18 22:40	melanie	Environment	=> Unknown
2008-11-19 10:37	HomerHorwitz	Relationship added	related to 0002611
2008-11-19 10:45	jhurliman	Note Added: 0007236
2008-11-24 15:14	jhurliman	File Added: openjpeg-dotnet-20081124.zip
2008-11-24 15:15	jhurliman	Note Added: 0007303
2008-11-24 15:15	jhurliman	Issue Monitored: jhurliman
2008-11-24 19:21	cfk	Note Added: 0007304
2008-11-25 08:07	justincc	Note Added: 0007315
2008-11-25 08:08	justincc	Status	new => patch included
2008-12-06 12:48	HomerHorwitz	Note Added: 0007606
2008-12-06 14:33	jhurliman	Note Added: 0007608
2008-12-16 12:21	jhurliman	Note Added: 0007995
2008-12-17 13:48	Fly-Man-	Note Added: 0008024
2008-12-19 12:06	HomerHorwitz	Note Added: 0008124
2008-12-19 12:36	justincc	Note Added: 0008126
2008-12-26 14:14	Teravus	Mono Version	=> None
2008-12-26 14:14	Teravus	Target Version	=> 0.6.2
2009-01-06 15:12	HomerHorwitz	File Added: check_image.patch
2009-01-06 15:12	HomerHorwitz	Note Added: 0008629
2009-01-06 16:08	nlin	Issue Monitored: nlin
2009-01-06 18:07	cfk	Note Added: 0008632
2009-01-06 18:18	jhurliman	Note Added: 0008633
2009-01-08 15:20	jhurliman	File Added: openjpeg-dotnet-20090108.zip
2009-01-08 15:21	jhurliman	Note Added: 0008678
2009-01-09 17:46	svn	Checkin
2009-01-09 17:46	svn	Note Added: 0008685
2009-01-09 17:51	dahlia	Note Added: 0008686
2009-01-09 17:51	dahlia	Assigned To	=> dahlia
2009-01-09 17:51	dahlia	Status	patch included => patch feedback
2009-01-10 01:22	dahlia	Note Added: 0008690
2009-01-11 12:36	Teravus	Note Added: 0008707
2009-01-11 12:49	Teravus	Note Edited: 0008707
2009-01-13 12:18	PocoLoco	Issue Monitored: PocoLoco
2009-01-21 03:25	Teravus	Status	patch feedback => resolved
2009-01-21 03:25	Teravus	Fixed in Version	=> 0.6.0+svn
2009-01-21 03:25	Teravus	Resolution	open => fixed
2009-01-21 03:25	Teravus	Note Added: 0008912
2009-01-25 09:17	Teravus	Status	resolved => closed

Notes
(0007236) jhurliman (manager) 2008-11-19 10:45	Several issues cropped up during testing. It appears that OpenJPEG by itself is not thread-safe, so decoding lots of textures at the same time without locking is bound for failure. The idea of JPEG2000 decoding (expensive) every time someone requests a texture is also a very bad move for performance, and ideally this texture metadata should be cached alongside images. The final issue that is a real killer is that it doesn't seem possible to trap segfaults in unmanaged code from Mono, only .NET. Because of this I'm investigating http://www.codeplex.com/csj2k/ [^] to see if the JPEG2000 implementation is complete enough to handle SL-style textures.

(0007303) jhurliman (manager) 2008-11-24 15:15	I attached openjpeg-dotnet-20081124.zip which includes a recompiled openjpeg-dotnet.dll (I was having problems with the previous compile for some reason) and the latest OpenMetaverse.dll which includes OpenJPEG locking and better error output for failed decodes.

(0007304) cfk (administrator) 2008-11-24 19:21	I would feel more comfortable if Teravus or Justin took this patch to commit as I dont want to fubar svn tonight.

(0007315) justincc (manager) 2008-11-25 08:07	Unfortunately we may have to move now to asking people to try this one experimentally before putting it in. I'll try and remember to ask if people can do this at the osgrid meeting today.

(0007606) HomerHorwitz (manager) 2008-12-06 12:48	Tried to apply it: - Doesn't apply cleanly to r7631. - Still contains the sandbox AppDomain code in XEngine After fixing that (hopefully) and applying it to r7631, textures load without problem here, as long as the textures are without problem. If I trash an texture-asset entry and try to load the texture, OpenSim crashes.

(0007608) jhurliman (manager) 2008-12-06 14:33	I need to look into catching the segfaults from C and returning an error code since Mono can't handle crashes in unmanaged code.

(0007995) jhurliman (manager) 2008-12-16 12:21	<jhurliman> Fly-Man-, the patch works fine as-is right now, unless you feed it bad texture data on mono <jhurliman> then it crashes, and after talking with the mono team i guess there is no way around that except creating a separate unmanaged process that does texture decoding

(0008024) Fly-Man- (developer) 2008-12-17 13:48	Patch failed: patching file prebuild.xml Reversed (or previously applied) patch detected! Assume -R? [n] Y Hunk 0000001 succeeded at 409 (offset -52 lines). Hunk 0000002 succeeded at 627 (offset -79 lines). Hunk 0000003 succeeded at 660 (offset -79 lines). Hunk 0000004 succeeded at 940 (offset 108 lines). Hunk 0000005 succeeded at 1195 (offset 58 lines). Hunk 0000006 succeeded at 1854 (offset -88 lines). Hunk 0000007 succeeded at 1895 (offset -88 lines). Hunk 0000008 FAILED at 2029. 1 out of 8 hunks FAILED -- saving rejects to file prebuild.xml.rej patching file OpenSim/Region/Environment/Scenes/Scene.Inventory.cs patching file OpenSim/Region/Environment/Modules/Agent/TextureSender/TextureSender.cs patching file OpenSim/Region/Environment/Modules/Agent/TextureDownload/TextureDownloadModule.cs Hunk 0000001 succeeded at 198 (offset 22 lines). patching file OpenSim/Region/Environment/Modules/Agent/TextureDownload/TextureNotFoundSender.cs patching file OpenSim/Region/Environment/Modules/Agent/TextureDownload/UserTextureDownloadService.cs patching file OpenSim/Region/Environment/Modules/Avatar/Friends/FriendsModule.cs Hunk 0000001 succeeded at 30 (offset 1 line). patching file OpenSim/Region/ScriptEngine/XEngine/XEngine.cs patching file OpenSim/Framework/Communications/Capabilities/LLSD.cs Reversed (or previously applied) patch detected! Assume -R? [n] Y

(0008124) HomerHorwitz (manager) 2008-12-19 12:06	Handling the segfaults via whatever method possible might be a workaround for our problem, but in my opinion openjpeg mustn't crash on bad data. So, the real fix can only be done on openjpeg.

(0008126) justincc (manager) 2008-12-19 12:36	Also, sdague brought up the very good point that if a texture can cause a segfault, then an artificially constructed texture could get a shell.

(0008629) HomerHorwitz (manager) 2009-01-06 15:12	I looked into the crash and found the reason for it (for this particular crash): The dotnet-wrapper contains some bugs while checking for successful image-decoding. The attached patch (check_image.patch) fixes this bug; after that I didn't manage to crash the region server with the progressive texture patch anymore.

(0008632) cfk (administrator) 2009-01-06 18:07	Well, are we ready to move forward and apply this patch? Does JHurliman need to do anything else?

(0008633) jhurliman (manager) 2009-01-06 18:18	We need to push out new openjpeg-dotnet binaries for all of the platforms. Thank you for the patch HomerHorwitz, I'll start compiling this evening or tomorrow.

(0008678) jhurliman (manager) 2009-01-08 15:21	New source and binaries are in the libomv trunk. I've also attached the binaries to this issue in a zip file for convenience. Drop these in OpenSim's trunk/bin/ and the progressive texture patch should be ready to go.

(0008685) svn (developer) 2009-01-09 17:46	dahlia committed related code in r8001 Thanks jhurliman for a patch that implements progressive texture downloading - Mantis 0002655 see more at - http://opensimulator.org/cgi-bin/viewvc.cgi?view=rev&revision=8001 [^]

(0008686) dahlia (manager) 2009-01-09 17:51	applied to r8001. Interested parties please test asap.

(0008690) dahlia (manager) 2009-01-10 01:22	reverted in r8004. Several issues appeared after commit that did not show up in initial pre-commit testing: 1. Script engines fail when running on Windows platform (see mantis 0002976) * commenting out the sandbox code in lines 552-558 appeared to resolve this 2. Texture downloads freeze avatar movement and can cause many console errors when logging into a texture rich region after clearing cache

(0008707) Teravus (administrator) 2009-01-11 12:36 edited on: 2009-01-11 12:49	This is probably a long held lock/possibly a deadlock caused by blockingqueue interaction with the texture send lock. For this to be effective, the decoding needs to be done outside a lock. Thread track: UDP Packet request texture---> Texture Request thread requests texture from assets--> Texture comes back and locks while openjpeg decodes---> UDP Packet request texture---> Texture is in cache and the first UDP packet has locked the object while decoding---> UDP processing client thread waits.... UDP processing thread does not respond to pings Client logs off.

(0008912) Teravus (administrator) 2009-01-21 03:25	Need to separate the decode thread from the client thread and cache the results, but this is in, and it's good!