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

fixed #17012: Loading assets synchronously causes rendering to freeze. #17464

Merged
merged 10 commits into from
Jul 31, 2024

Conversation

dumganhar
Copy link
Contributor

@dumganhar dumganhar commented Jul 31, 2024

The main thread will not be freezed now and the performance on android is better than before.

Re: #17012

cocos/cocos-engine-external#455

Performance

Devices Loading 200+ json files (size: 90MB) 3 times (Before), unit: milliseconds Loading 200+ json files (size: 90MB) 3 times (Now), unit: milliseconds
SM-J400G (arm32, android 8.0) 4346
4324
4326
2698
3194
2877
Pixel 4XL (arm64, android 13) 971
964
975
699
662
713
SM-J111F (arm32, android 5.1) 5871
5939
5622
3662
3983
3704
iphone X (arm64, iOS 16.7.2) 696
664
647
696
708
695
ipad 6 (arm64, iOS 12.2) 674
755
717
736
738
729
mac m1pro (arm64, macOS 14.5) 359
424
404
410
452
408

Changelog


Continuous Integration

This pull request:

  • needs automatic test cases check.

    Manual trigger with @cocos-robot run test cases afterward.

  • does not change any runtime related code or build configuration

    If any reviewer thinks the CI checks are needed, please uncheck this option, then close and reopen the issue.


Compatibility Check

This pull request:

  • changes public API, and have ensured backward compatibility with deprecated features.
  • affects platform compatibility, e.g. system version, browser version, platform sdk version, platform toolchain, language version, hardware compatibility etc.
  • affects file structure of the build package or build configuration which requires user project upgrade.
  • introduces breaking changes, please list all changes, affected features and the scope of violation.

Greptile Summary

Introduced asynchronous asset loading to prevent rendering freezes, significantly improving performance on Android devices.

  • native/cocos/bindings/manual/jsb_global.cpp: Added async image loading and saving functions, leveraging a thread pool.
  • native/cocos/bindings/jswrapper/v8/Utils.h: Enhanced ExternalStringResource with a Dispose method for better memory management.
  • native/cocos/bindings/manual/jsb_cocos_manual.cpp: Implemented async asset loading to avoid rendering freezes, focusing on thread pool tasks and JSON parsing.
  • native/external-config.json: Updated to reference v3.8.4-3 of the external engine-native repository.
  • cocos/native-binding/index.ts: Introduced async file reading functions (readTextFile, readJsonFile, readDataFile).

@dumganhar dumganhar requested a review from minggo July 31, 2024 10:13
Copy link

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

14 file(s) reviewed, no comment(s)
Edit PR Review Bot Settings

Copy link

github-actions bot commented Jul 31, 2024

Interface Check Report

! WARNING this pull request has changed these public interfaces:

@@ -34479,8 +34479,29 @@
              *
              */
             function getStringFromFile(filename: string): string;
             /**
+             * @en Read utf-8 text file asynchronously.
+             * @zh 异步读取 utf-8 编码的文本文件
+             * @param filepath @en The file path. @zh 文件路径
+             * @param onComplete @en The complete callback. @zh 读取完成回调
+             */
+            function readTextFile(filepath: string, onComplete: (err: string | null, content: string) => void): void;
+            /**
+             * @en Read utf-8 json file asynchronously.
+             * @zh 异步读取 utf-8 编码的 json 文件
+             * @param filepath @en The file path. @zh 文件路径
+             * @param onComplete @en The complete callback. @zh 读取完成回调
+             */
+            function readJsonFile(filepath: string, onComplete: (err: string | null, content: object) => void): void;
+            /**
+             * @en Read binary file asynchronously.
+             * @zh 异步读取二进制文件
+             * @param filepath @en The file path. @zh 文件路径
+             * @param onComplete @en The complete callback. @zh 读取完成回调
+             */
+            function readDataFile(filepath: string, onComplete: (err: string | null, content: ArrayBuffer) => void): void;
+            /**
              *  @en
              *  Removes a file.
              *
              *  @zh

Copy link

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

2 file(s) reviewed, no comment(s)
Edit PR Review Bot Settings

@@ -1526,7 +1526,7 @@ SE_BIND_FUNC(JSB_openharmony_postSyncMessage)
#endif

bool jsb_register_global_variables(se::Object *global) { // NOLINT
gThreadPool = LegacyThreadPool::newFixedThreadPool(3);
gIOThreadPool = LegacyThreadPool::newFixedThreadPool(5);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There may be lots of json/ccb files to be loaded, so add 2 more threads in pool.

Copy link

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

1 file(s) reviewed, no comment(s)
Edit PR Review Bot Settings

Copy link

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

2 file(s) reviewed, no comment(s)
Edit PR Review Bot Settings

Copy link

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

1 file(s) reviewed, no comment(s)
Edit PR Review Bot Settings

Copy link

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

1 file(s) reviewed, no comment(s)
Edit PR Review Bot Settings

@dumganhar
Copy link
Contributor Author

@cocos-robot run test cases

Copy link

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

74 file(s) reviewed, no comment(s)
Edit PR Review Bot Settings

Copy link

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

4 file(s) reviewed, no comment(s)
Edit PR Review Bot Settings

Copy link

@dumganhar, Please check the result of run test cases:

Task Details

Copy link

@dumganhar, Please check the result of run test cases:

Task Details

Platform build boot runned crashScene FailScene
windows PASS PASS PASS
android PASS PASS PASS
wechatgame FAIL FAIL

@dumganhar dumganhar merged commit ab6b7bb into cocos:v3.8.4 Jul 31, 2024
24 checks passed
dumganhar added a commit to dumganhar/cocos-engine that referenced this pull request Aug 15, 2024
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.

2 participants