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

[gapid2apk] add standalone APK creation to export_replay #2621

Merged
merged 2 commits into from
Apr 19, 2019

Conversation

hevrard
Copy link
Contributor

@hevrard hevrard commented Feb 21, 2019

This is all the code needed for gapid2apk on the gapir side: detect replay resources in the apk assets, and if present use them to do the replay.

The missing part is the external command to actually create a new APK with replay resources: I have this implemented as a bash script, but we will want it as a regular command in Go for the sake or portability.

I open the PR now for reviews of the GAPIR side of things, and I will add the APK creation command soon.

@ben-clayton
Copy link
Contributor

ben-clayton commented Feb 22, 2019

Please ensure you squash these changes before submitting. Given that nobody has started reviewing yet, perhaps now would be a good time to squash to one or a few CLs?

<edit> I got the impression this was close to final review, but I've seen in chat that you still have more stuff to do, so ignore me. :) </edit>

Copy link
Contributor

@ben-clayton ben-clayton left a comment

Choose a reason for hiding this comment

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

Looking pretty good so far. Good stuff!

cmd/gapit/trace2apk.go Outdated Show resolved Hide resolved
cmd/gapit/trace2apk.go Outdated Show resolved Hide resolved
cmd/gapit/trace2apk.go Outdated Show resolved Hide resolved
cmd/gapit/trace2apk.go Outdated Show resolved Hide resolved
cmd/gapit/trace2apk.go Outdated Show resolved Hide resolved
cmd/gapit/trace2apk.go Outdated Show resolved Hide resolved
@AWoloszyn
Copy link
Contributor

FYI: This may be a future update, but I tried this, and the performance was rather abysmal. From what I could tell, almost all of the time was spend unzipping resources from the archive. (Which is somewhat expected).

@hevrard hevrard force-pushed the gapid2apk branch 3 times, most recently from 130cf07 to e9385e6 Compare March 13, 2019 14:48
cmd/gapit/trace2apk.go Outdated Show resolved Hide resolved
cmd/gapit/trace2apk.go Outdated Show resolved Hide resolved
cmd/gapit/trace2apk.go Outdated Show resolved Hide resolved
cmd/gapit/trace2apk.go Outdated Show resolved Hide resolved
cmd/gapit/trace2apk.go Outdated Show resolved Hide resolved
@hevrard hevrard force-pushed the gapid2apk branch 2 times, most recently from 1434b2f to 366e9cc Compare April 3, 2019 11:36
@hevrard hevrard changed the title WIP gapid2apk Add standalone APK creation to export_replay (gapid2apk) Apr 3, 2019
@hevrard
Copy link
Contributor Author

hevrard commented Apr 3, 2019

Thanks for the previous review, I tried to fix all remarks. This PR enables to create a stand-alone APK which replay a given trace. We are still facing issues when trying to trace the created APKs, but we can fix these issues later.

As it is a big change, we may think of splitting this in a few PRs, feel free to suggest such splitting.

@hevrard hevrard requested review from AWoloszyn and pmuetschard April 3, 2019 12:24
@hevrard hevrard changed the title Add standalone APK creation to export_replay (gapid2apk) [gapid2apk] add standalone APK creation to export_replay Apr 3, 2019
cmd/gapir/cc/main.cpp Outdated Show resolved Hide resolved
cmd/gapir/cc/main.cpp Outdated Show resolved Hide resolved
cmd/gapir/cc/main.cpp Outdated Show resolved Hide resolved
cmd/gapit/export_replay.go Outdated Show resolved Hide resolved
cmd/gapit/export_replay.go Outdated Show resolved Hide resolved
cmd/gapit/export_replay.go Outdated Show resolved Hide resolved
cmd/gapit/export_replay.go Outdated Show resolved Hide resolved
cmd/gapit/flags.go Outdated Show resolved Hide resolved
gapis/api/gles/api/egl.api Outdated Show resolved Hide resolved
@AWoloszyn
Copy link
Contributor

Other than the comments from @pmuetschard it looks good to me.

The APK is created from the regular GAPID APK by addind replay export
files as assets, and GAPIR detects those files and replay from them.
@hevrard
Copy link
Contributor Author

hevrard commented Apr 5, 2019

Many thanks Pascal for the detailed feedback!
The EGL frame delimiters replay is now separated in #2708

@hevrard
Copy link
Contributor Author

hevrard commented Apr 9, 2019

The OSX build fails with ERROR: Aborting VM command due to timeout of 10800 seconds, have you seen that before? How can we allocate more time for the build?

@hevrard hevrard merged commit fa041f6 into google:master Apr 19, 2019
@hevrard hevrard deleted the gapid2apk branch April 19, 2019 16:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants