Skip to content

Comments

[gapid2apk] add standalone APK creation to export_replay#2621

Merged
hevrard merged 2 commits intogoogle:masterfrom
hevrard:gapid2apk
Apr 19, 2019
Merged

[gapid2apk] add standalone APK creation to export_replay#2621
hevrard merged 2 commits intogoogle:masterfrom
hevrard:gapid2apk

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 •
edited
Loading

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>

ben-clayton
ben-clayton reviewed Feb 22, 2019
View reviewed changes
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!

AWoloszyn
AWoloszyn reviewed Feb 25, 2019
View reviewed changes
AWoloszyn
AWoloszyn reviewed Feb 25, 2019
View reviewed changes
AWoloszyn
AWoloszyn reviewed Feb 25, 2019
View reviewed changes
AWoloszyn
AWoloszyn reviewed Feb 25, 2019
View reviewed changes
pau-baiget
pau-baiget reviewed Feb 28, 2019
View reviewed changes
@AWoloszyn
Copy link
Contributor

AWoloszyn commented Mar 5, 2019

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
AWoloszyn
AWoloszyn reviewed Mar 13, 2019
View reviewed changes
AWoloszyn
AWoloszyn reviewed Mar 13, 2019
View reviewed changes
AWoloszyn
AWoloszyn reviewed Mar 13, 2019
View reviewed changes
AWoloszyn
AWoloszyn reviewed Mar 13, 2019
View reviewed changes
AWoloszyn
AWoloszyn reviewed Mar 13, 2019
View reviewed changes
AWoloszyn
AWoloszyn reviewed Mar 26, 2019
View reviewed changes
AWoloszyn
AWoloszyn reviewed Mar 26, 2019
View reviewed changes
AWoloszyn
AWoloszyn reviewed Mar 26, 2019
View reviewed changes
AWoloszyn
AWoloszyn reviewed Mar 26, 2019
View reviewed changes
AWoloszyn
AWoloszyn reviewed Mar 26, 2019
View reviewed changes
AWoloszyn
AWoloszyn reviewed Mar 26, 2019
View reviewed changes
@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
pmuetschard
pmuetschard reviewed Apr 3, 2019
View reviewed changes
@AWoloszyn
Copy link
Contributor

AWoloszyn commented Apr 4, 2019

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

@hevrard
Add standalone APK creation option in export_replay
f81a7c1 
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 hevrard requested a review from pmuetschard April 5, 2019 20:36
@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

Reviewers

@pmuetschard pmuetschard Awaiting requested review from pmuetschard

3 more reviewers

@ben-clayton ben-clayton ben-clayton left review comments

@pau-baiget pau-baiget pau-baiget left review comments

@AWoloszyn AWoloszyn AWoloszyn approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

cla: yes

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

@hevrard @ben-clayton @AWoloszyn @pmuetschard @pau-baiget @googlebot @kokoro-team