Content-Length: 328564 | pFad | https://github.com/NativeScript/NativeScript/issues/10402

DFB perf(android): gridlayout with less JNI calls by farfromrefug · Pull Request #10402 · NativeScript/NativeScript · GitHub
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.

Sign up for GitHub

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

Jump to bottom

perf(android): gridlayout with less JNI calls #10402

Merged
merged 10 commits into from
Jul 1, 2024
Merged

perf(android): gridlayout with less JNI calls #10402

merged 10 commits into from
Jul 1, 2024

Conversation

farfromrefug
Copy link
Collaborator

A rewrite of GridLayout on Android to makes less JNI calls. The result is around 30% time gained on average GridLayout

@cla-bot cla-bot bot added the cla: yes label Oct 10, 2023
@NathanWalker NathanWalker added this to the 8.7 milestone Oct 10, 2023
@NathanWalker NathanWalker modified the milestones: 8.7, 8.8 Jun 28, 2024
@NathanWalker NathanWalker changed the title fix(android): rewrite the GridLayout to make as less JNI calls as pos… perf(android): gridlayout with less JNI calls Jun 28, 2024
@NathanWalker
Copy link
Contributor

@farfromrefug possible to peek at the failing android tests? I merged latest and also rebuild the ui-mobile-base widgets (aka, TNSWidgets) since this has a .java change on it) and seems we have the following failing tests with these changes:

 Test: GRIDLAYOUT.test_removeColumns FAILED: Cannot convert number to Lorg/nativescript/widgets/GridUnitType; at index 1
 Test: GRIDLAYOUT.test_removeRows FAILED: Cannot convert number to Lorg/nativescript/widgets/GridUnitType; at index 1
 Test: GRIDLAYOUT.test_removeChildren FAILED: Cannot convert number to Lorg/nativescript/widgets/GridUnitType; at index 1
 Test: GRIDLAYOUT.test_measuredWidth_when_not_stretched_two_columns FAILED: Cannot convert number to Lorg/nativescript/widgets/GridUnitType; at index 1
 Test: GRIDLAYOUT.test_measuredWidth_when_not_stretched_three_columns FAILED: Cannot convert number to Lorg/nativescript/widgets/GridUnitType; at index 1
 Test: GRIDLAYOUT.test_desiredSize_isCorrect FAILED: Cannot convert number to Lorg/nativescript/widgets/GridUnitType; at index 1
 Test: GRIDLAYOUT.test_columnsActualWidth_isCorrect FAILED: Cannot convert number to Lorg/nativescript/widgets/GridUnitType; at index 1
 Test: GRIDLAYOUT.test_rowsActualHeight_isCorrect FAILED: Cannot convert number to Lorg/nativescript/widgets/GridUnitType; at index 1
 Test: GRIDLAYOUT.test_Measure_and_Layout_Children_withCorrect_size FAILED: Cannot convert number to Lorg/nativescript/widgets/GridUnitType; at index 1
 Test: GRIDLAYOUT.test_ColumnWidth_when_4stars_and_width_110 FAILED: Cannot convert number to Lorg/nativescript/widgets/GridUnitType; at index 1
 Test: GRIDLAYOUT.test_columns_widths FAILED: Cannot convert number to Lorg/nativescript/widgets/GridUnitType; at index 1

Link to test run here: https://github.com/NativeScript/NativeScript/actions/runs/9719521521/job/26829475734?pr=10402

@farfromrefug
Copy link
Collaborator Author

@NathanWalker will take a look

@farfromrefug
Copy link
Collaborator Author

@NathanWalker i did fix some tests. now i am facing a dilemma. Some tests like this one https://github.com/Akylas/NativeScript/blob/b9d5ba1c53987a7bbc3a8430b327c96dac03eb8c/apps/automated/src/ui/layouts/grid-layout-tests.ts#L372 are still failing. The reason is that before my PR on the JS side _cols and _rows were actually native objects modified from the native side (org.nativescript.widgets.ItemSpec). Then on those native objects _actualLength to assert the tests.
But that only existed for the tests and makes GridLayout slower as we maintain and access native objects from the JS side. So i dont think we should keep on doing it that way.
I can create special methods in the android GridLayout to query that _actualLength for the purpose of the tests. However it means the tests will be different on iOS / Android. IS that ok?

@NathanWalker
Copy link
Contributor

Thanks for explaining, I think that’d be okay 👍

@farfromrefug
Copy link
Collaborator Author

@NathanWalker should be good now

@NathanWalker NathanWalker merged commit 6dd441d into NativeScript:main Jul 1, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@NathanWalker NathanWalker

Assignees
No one assigned
Labels
cla: yes
Projects
None yet
Milestone
8.8
Development

Successfully merging this pull request may close these issues.
2 participants
@farfromrefug @NathanWalker








ApplySandwichStrip

pFad - (p)hone/(F)rame/(a)nonymizer/(d)eclutterfier!      Saves Data!


--- a PPN by Garber Painting Akron. With Image Size Reduction included!

Fetched URL: https://github.com/NativeScript/NativeScript/issues/10402

Alternative Proxies:

Alternative Proxy

pFad Proxy

pFad v3 Proxy

pFad v4 Proxy