-
Notifications
You must be signed in to change notification settings - Fork 454
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
Some skia APIs deadlock on android app when using react-native-v8 #1705
Comments
I've uncovered a little bit more info. I got the android studio debugger attached and when the app is hung it always pauses on //in SkiaDomView.java:41
protected native void registerView(int nativeId); Not sure if that's useful but at least it's a clue. |
I'm at the end of my ability to debug this further I think. @chrfalch @wcandillon does anything obvious jump out at you as the cause of this deadlock? |
Hi @evelant! Thanks for the insights about running RN Skia with RN V8. We've not yet done any explicit tests running react-native-v8 - and we don't have any direct requests for this meaning that we'll probl. not be able to prioritise this for now. I've seen that @Kudo is already giving great feedback on this - we'll wait to see if he can reproduce and see the error before moving on on our side. |
I figured that was the case 🙂 I'll hopefully have a bit of time tomorrow to reproduce this and will work with @Kudo on it. I filed this bug just for your awareness and for the off chance that the problem might immedidately jump out to you. Thanks for all your hard work on this awesome lib! |
I think I found the root cause. Reproduction here: https://github.com/evelant/rnv8skiacrash tl;dr -- v8 crashes or deadlocks when provided with invalid props such as NaN, hermes and JSC don't. I don't really know how JSI works internally but I'm guessing this means it's likely a fix needed in react-native-v8, not skia. |
It seems I may have been mistaken -- the issue actually appears to be something deeper involving |
@evelant This may not be surprising. Did you switch your own loading to load fonts? const data = await Skia.Data.fromURI(require(./font.ttf));
const font Skia.Font(Skia.Typeface.MakeFreeTypeFaceFromData(data), size)`; I'd be curious if this works for you. If it does, we could document it and even look at improving our data loading hooks. |
@chrfalch I haven't got anything to contribute to the ticket, but I just wanted to share that v8 is used for the project I'm working on because of the performance increase it gives. So while there might not be much vocal support behind v8, I think it would be something really worth while testing against as people who use Skia will have performance in mind and will probably want to run it in v8. |
@wcandillon Sorry to take so long to get back to you, I was on vacation. I'm not quite understanding what you mean -- I already use |
@evelant because we have a bug in the useFont possibily |
@wcandillon OK I tried loading the font manually like you specified above. Unfortunately the behavior is the same. My app still deadlocks on v8 in the same place as the stack traces above. |
Description
I'm attempting to switch my app from hermes to v8 on android since v8 is 4-5x faster than hermes for me. I must be tripping over some nasty hermes perf corner case. In doing so I discovered that some skia views cause the app to hang when running on v8.
Unfortunately I haven't been able to pinpoint exactly the conditions that create this yet. It seems that in some cases a component containing even an empty can hang the app. A lot of other uses of Canvas work perfectly fine however.
I'm not quite sure how to debug this. It seems likely to me that there's some difference between how hermes and v8 handle JSI libraries that's causing the issue, but I lack the knowledge to attempt to figure out what that could be.
I'm sorry this issue is vague and doesn't have a good reproduction yet. I'm attempting to narrow it down. Until then, can anybody offer some pointers on where I might start looking?
issue filed on react-native-v8: Kudo/react-native-v8#186
Version
0.1.197
Steps to reproduce
Snack, code example, screenshot, or link to a repository
Unfortunately I don't have one yet. I'm working on narrowing down the issue.
Environment:
"@shopify/react-native-skia": "0.1.197",
"expo": "~49.0.0",
"react-native": "0.72.1",
"react-native-v8": "2.2.2"
"v8-android-jit": "11.1000.3"
If it's relevant, I do not have reanimated installed.
The text was updated successfully, but these errors were encountered: