Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use LZ4 as external dependency #1566

Merged
merged 1 commit into from
Jan 14, 2023
Merged

Conversation

Jklawreszuk
Copy link
Collaborator

PR Details

This PR removes LZ4 compression algorithm from Stride repo and uses as a external package.

Related Issue

#1394

Types of changes

  • Docs change / refactoring / dependency upgrade
  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist

  • My change requires a change to the documentation.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

@manio143
Copy link
Member

Looks good. Have you tested if it's actually compatible or breaking on compiled assets (build game before the change, copy generated assets from output, rebuild after the change, use copied assets and run game)?

@Jklawreszuk
Copy link
Collaborator Author

Jklawreszuk commented Nov 21, 2022

After longer testing I've found that Stride.Assets.Compiler throws error that 'Could not load file or assembly 'K4os.Compression.LZ4.Legacy' which is weird.
logs here : error.txt

I don't know why this happened, but now is working perfectly fine.

@Jklawreszuk
Copy link
Collaborator Author

@manio143 I've tested the changes on a couple of projects and I didn't notice any regression. However, our LZ4Stream implementation must stay, because as it turned out K4os version is not quite complete :/ (e.g doesn't have an implementation of the Seek method or doesn't remember current position value). Otherwise I guess is fine

@manio143 manio143 merged commit 741b3af into stride3d:master Jan 14, 2023
@manio143
Copy link
Member

Thank you for checking! Merging this PR is a great step towards removing native compilation in Stride.

@Jklawreszuk Jklawreszuk deleted the lz4-package branch January 14, 2023 22:16
@skohanowski
Copy link
Contributor

skohanowski commented Jan 17, 2023

After longer testing I've found that Stride.Assets.Compiler throws error that 'Could not load file or assembly 'K4os.Compression.LZ4.Legacy' which is weird. logs here : error.txt

I don't know why this happened, but now is working perfectly fine.

After sync'ing I ran into this. Something I did with the nuget sources seemed to resolve it though, and I'm having trouble reproducing it now.

  • Multiple rebuilds on their own didn't have an affect.
  • Then I dropped "MSBuildSdkExtras CI Builds" as a source and rebuilt and everything started working.
  • I reset, cleaned up the repo and cleared out my local package caches to reproduce the error and now I can't.

The issue only occurred when starting a game, not while building. I'm using the local stride dev package source for my game. Since I hit this first thing after sync'ing I'm betting others will too.

I'm still looking into it.

@skohanowski
Copy link
Contributor

Well, I did these steps:

  1. deleted all the obj and bins folders of my projects
  2. cleared out all my nuget caches
  3. reverted my stride repo to prior to the sync
  4. cleaned up it's non repo files
  5. built and ran the game
  6. brought master back to head
  7. rebuilt stride
  8. built and ran the game again

I still can't seem to reproduce the error.
I suspect it's not package source but like manio143 asked about above regarding cached asset compilations. If someone else runs into this you can probably get around it by deleting your game obj and bins and rebuilding but that's just a guess.

@manio143
Copy link
Member

If the assembly load error occured at game run it suggests at the time the dll wasn't copied over and rebuilding should have fixed it. I'm not sure why but I have a hunch that it's a caching issue around NuGet - not having a repro we can't pin it down, but it shouldn't occur for users once the new packages are released as they will be downloading fresh packages with an increased version, this skipping the cache.

@joe88ds
Copy link

joe88ds commented Jan 18, 2023

2>EXEC : error 4.907s: [AssetCompiler] Unhandled exception. Exception: FileNotFoundException: Could not load file or assembly 'K4os.Compression.LZ4, Version=1.2.16.0, Culture=neutral, PublicKeyToken=2186fa9121ef231d'. The system cannot find the file specified.
2>   at Stride.Core.LZ4.LZ4Stream.FlushCurrentChunk()
2>   at Stride.Core.LZ4.LZ4Stream.Flush() in C:\Users\x\Documents\Repos\Stride\stride\sources\core\Stride.Core.Serialization\Serialization\LZ4\LZ4Stream.cs:line 334
2>   at Stride.Core.Storage.BundleOdbBackend.CreateBundle(String bundleUrl, IOdbBackend backend, ObjectId[] objectIds, ISet`1 disableCompressionIds, Dictionary`2 indexMap, IList`1 dependencies, Boolean useIncrementalBundle) in C:\Users\x\Documents\Repos\Stride\stride\sources\core\Stride.Core.Serialization\Storage\BundleOdbBackend.cs:line 675
2>   at Stride.Core.Storage.ObjectDatabase.CreateBundle(ObjectId[] objectIds, String bundleName, BundleOdbBackend bundleBackend, ISet`1 disableCompressionIds, Dictionary`2 indexMap, IList`1 dependencies, Boolean useIncrementalBundle) in C:\Users\x\Documents\Repos\Stride\stride\sources\core\Stride.Core.Serialization\Storage\ObjectDatabase.cs:line 112
2>   at Stride.Core.Assets.CompilerApp.BundlePacker.Build(Logger logger, PackageSession packageSession, Package rootPackage, String indexName, String outputDirectory, ISet`1 disableCompressionIds, Boolean useIncrementalBundles, List`1 bundleFiles) in C:\Users\x\Documents\Repos\Stride\stride\sources\assets\Stride.Core.Assets.CompilerApp\BundlePacker.cs:line 241
2>   at Stride.Core.Assets.CompilerApp.PackageBuilder.BuildMaster() in C:\Users\x\Documents\Repos\Stride\stride\sources\assets\Stride.Core.Assets.CompilerApp\PackageBuilder.cs:line 169
2>   at Stride.Core.Assets.CompilerApp.PackageBuilder.Build() in C:\Users\x\Documents\Repos\Stride\stride\sources\assets\Stride.Core.Assets.CompilerApp\PackageBuilder.cs:line 57
2>   at Stride.Core.Assets.CompilerApp.PackageBuilderApp.Run(String[] args) in C:\Users\x\Documents\Repos\Stride\stride\sources\assets\Stride.Core.Assets.CompilerApp\PackageBuilderApp.cs:line 304

I fixed the issue by copying: 'C:\Users\x.nuget\packages\k4os.compression.lz4\1.2.16\lib\net5.0\K4os.Compression.LZ4.dll' into 'C:\Users\x.nuget\packages\stride.core.serialization\4.1.0.1\lib\net6.0'.

Steps to reproduce:

  1. git clone https://github.com/stride3d/stride.git
  2. Open Stride.sln
  3. Build Stride.GameStudio
  4. Create template: top-down RPG
  5. Open the .sln file from Visual Studio 2022
  6. Debug -> Start Debugging

At this point, the build error should occur.

The main issue is the K4os.Compression.LZ4.dll isn't being packed with Stride.Core.Serialization.

@manio143
Copy link
Member

I see that missing DLL is a dependency of the Legacy package being referenced - could it be there's something blocking transitive dependency inclusion? But why would the issue get fixed some of the time 🤔

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants