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

Optimize PackageInfoService #1136

Merged
merged 10 commits into from
Sep 27, 2017
3 changes: 3 additions & 0 deletions cmd/gapit/packages.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import (
"github.com/google/gapid/core/app"
"github.com/google/gapid/core/log"
"github.com/google/gapid/core/os/android/adb"
"github.com/google/gapid/core/os/device/bind"
"github.com/google/gapid/core/os/file"
"github.com/google/gapid/gapidapk"
)
Expand All @@ -45,6 +46,8 @@ func init() {
}

func (verb *packagesVerb) Run(ctx context.Context, flags flag.FlagSet) error {
ctx = bind.PutRegistry(ctx, bind.NewRegistry())

if verb.ADB != "" {
adb.ADB = file.Abs(verb.ADB)
}
Expand Down
3 changes: 3 additions & 0 deletions cmd/gapit/trace.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ import (
"github.com/google/gapid/core/os/android"
"github.com/google/gapid/core/os/android/adb"
"github.com/google/gapid/core/os/android/apk"
"github.com/google/gapid/core/os/device/bind"
"github.com/google/gapid/core/os/file"
"github.com/google/gapid/core/os/process"
"github.com/google/gapid/core/os/shell"
Expand Down Expand Up @@ -59,6 +60,8 @@ const vulkanAPI = uint32(1 << 2)
const gvrAPI = uint32(1 << 3)

func (verb *traceVerb) Run(ctx context.Context, flags flag.FlagSet) error {
ctx = bind.PutRegistry(ctx, bind.NewRegistry())

options := client.Options{
ObserveFrameFrequency: uint32(verb.Observe.Frames),
ObserveDrawFrequency: uint32(verb.Observe.Draws),
Expand Down
10 changes: 5 additions & 5 deletions core/os/android/adb/forward_and_connect.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,9 +29,9 @@ import (
)

const (
connectionAttempts = 30
reconnectDelay = time.Second
ErrServiceTimeout = fault.Const("Timeout connecting to service")
reconnectDelay = time.Millisecond * 100
connectionTimeout = time.Second * 30
ErrServiceTimeout = fault.Const("Timeout connecting to service")
)

// ForwardAndConnect forwards the local-abstract-socket las and connects to it.
Expand All @@ -54,7 +54,8 @@ func ForwardAndConnect(ctx context.Context, d Device, las string) (io.ReadCloser

app.AddCleanup(ctx, unforward)

for i := 0; i < connectionAttempts; i++ {
start := time.Now()
for time.Since(start) < connectionTimeout {
if sock, err := net.Dial("tcp", fmt.Sprintf("localhost:%v", port)); err == nil {
reader := bufio.NewReader(sock)
if _, err := reader.Peek(1); err == nil {
Expand All @@ -65,7 +66,6 @@ func ForwardAndConnect(ctx context.Context, d Device, las string) (io.ReadCloser

return readerCustomCloser{reader, close}, nil
}

sock.Close()
}
time.Sleep(reconnectDelay)
Expand Down
24 changes: 23 additions & 1 deletion core/os/android/adb/installed_package.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,8 @@ import (
"github.com/google/gapid/core/os/device"
)

// InstalledPackages returns the sorted list of installed packages on the device.
// InstalledPackages returns the sorted list of installed packages on the
// device.
func (b *binding) InstalledPackages(ctx context.Context) (android.InstalledPackages, error) {
str, err := b.Shell("dumpsys", "package").Call(ctx)
if err != nil {
Expand All @@ -36,6 +37,27 @@ func (b *binding) InstalledPackages(ctx context.Context) (android.InstalledPacka
return b.parsePackages(str)
}

// InstalledPackage returns information about a single installed package on the
// device.
func (b *binding) InstalledPackage(ctx context.Context, name string) (*android.InstalledPackage, error) {
str, err := b.Shell("dumpsys", "package", name).Call(ctx)
if err != nil {
return nil, log.Errf(ctx, err, "Failed to get installed packages")
}
packages, err := b.parsePackages(str)
if err != nil {
return nil, err
}
switch len(packages) {
case 0:
return nil, fmt.Errorf("Package '%v' not found", name)
case 1:
return packages[0], nil
default:
return nil, fmt.Errorf("%v packages found with name '%v'", len(packages), name)
}
}

// The minSdk field was added more recently [see https://goo.gl/UN7oFv]
var reVersionCodeMinSDKTargetSDK = regexp.MustCompile("^(?:versionCode=([0-9]+))(?: minSdk=([0-9]+))? (?:targetSdk=([0-9]+))?.*$")

Expand Down
2 changes: 2 additions & 0 deletions core/os/android/device.go
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,8 @@ type Device interface {
GetScreenDimensions(ctx context.Context) (orientation, width, height int, ok bool)
// InstalledPackages returns the sorted list of installed packages on the device.
InstalledPackages(ctx context.Context) (InstalledPackages, error)
// InstalledPackage returns information about a single installed package on the device.
InstalledPackage(ctx context.Context, name string) (*InstalledPackage, error)
// IsScreenOn returns true if the device's screen is currently on.
IsScreenOn(ctx context.Context) (bool, error)
// TurnScreenOn turns the device's screen on.
Expand Down
4 changes: 4 additions & 0 deletions gapidapk/CMakeBuild.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,11 @@

set(android_sources
${CMAKE_CURRENT_SOURCE_DIR}/android/app/build.gradle
${CMAKE_CURRENT_SOURCE_DIR}/android/app/src/main/java/com/google/android/gapid/Cache.java
${CMAKE_CURRENT_SOURCE_DIR}/android/app/src/main/java/com/google/android/gapid/Counter.java
${CMAKE_CURRENT_SOURCE_DIR}/android/app/src/main/java/com/google/android/gapid/DeviceInfoService.java
${CMAKE_CURRENT_SOURCE_DIR}/android/app/src/main/java/com/google/android/gapid/FileCache.java
${CMAKE_CURRENT_SOURCE_DIR}/android/app/src/main/java/com/google/android/gapid/IOUtils.java
${CMAKE_CURRENT_SOURCE_DIR}/android/app/src/main/java/com/google/android/gapid/PackageInfoService.java
${CMAKE_CURRENT_SOURCE_DIR}/android/app/src/main/java/com/google/android/gapid/SocketWriter.java
${CMAKE_CURRENT_SOURCE_DIR}/android/app/src/main/res/drawable-xxxhdpi/logo.png
Expand Down
5 changes: 5 additions & 0 deletions gapidapk/android/.gitignore
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
# Ignore files generated by Android Studio
.gradle/**
.idea/**
local.properties
*.iml
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
/*
* Copyright (C) 2017 Google Inc.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package com.google.android.gapid;

import java.util.HashMap;
import java.util.Map;

/**
* Cache is an in-memory key-value store. Values are generated by the {@link Cache.Builder} when
* there is a cache-miss.
* @param <K> the cache key type.
* @param <V> the cache value type.
*/
public class Cache<K, V> {
private final Map<K, V> map = new HashMap<>();
private final Builder<K, V> builder;

/**
* Builder generates new values when there is a cache miss.
* @param <K> the cache key type.
* @param <V> the cache value type.
*/
interface Builder<K, V> {
V build(K key);
}

public static <K, V> Cache<K, V> create(Builder<K, V> builder) {
return new Cache<>(builder);
}

/**
* Lookups the value from the cache using the given key. If the cache does not contain the value
* then the {@link Builder} will be used to create the value.
Copy link
Member

Choose a reason for hiding this comment

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

Add a comment that this is not thread safe.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Made thread safe instead.

*/
public V get(K key) {
synchronized (map) {
if (map.containsKey(key)) {
return map.get(key);
}
}
V value = builder.build(key);
synchronized (map) {
map.put(key, value);
}
return value;
}

private Cache(Builder<K, V> builder) {
this.builder = builder;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,173 @@
/*
* Copyright (C) 2017 Google Inc.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package com.google.android.gapid;

import java.util.ArrayList;
import java.util.Collections;
import java.util.Comparator;
import java.util.HashMap;
import java.util.List;
import java.util.Map;

/**
* Counter is a basic multi-threaded hierarchical scope based profiler.
*/
public class Counter {
private static final Counter ROOT = new Counter("<root>", null);
private static final ThreadLocal<Counter> CURRENT = new ThreadLocal<>();

private final Map<String, Counter> children = new HashMap<>();
private final String name;
private final Counter parent;
private int count = 0;
private float time = 0;

/**
* time is used to profile the amount of time spent in the given scope, identified by the given
* name.
*
* It is to be used with a try-with-resources block:
* <code>
* try (Counter.Scope t = Counter.time("doSomething")) {
* doSomething();
* }
* </code>
*
* @param name the scope name.
* @return an {@link AutoCloseable} scope.
*/
public static Scope time(String name) {
Counter counter = CURRENT.get();
if (counter == null) {
counter = ROOT.child("Thread " + Thread.currentThread().getName());
CURRENT.set(counter);
}
return new Scope(counter.child(name));
}

/**
* Collect returns the timing information about all counters, across all threads.
* If reset is true, then the counters will be all cleared back to 0.
*/
public static String collect(boolean reset) {
StringBuilder sb = new StringBuilder();
synchronized (ROOT) {
for (Counter thread : ROOT.children.values()) {
thread.write(sb, 0);
}
}
if (reset) {
ROOT.reset();
}
return sb.toString();
}

private Counter(String name, Counter parent) {
this.name = name;
this.parent = parent;
}

private synchronized Counter child(String name) {
Counter counter = children.get(name);
if (counter == null) {
counter = new Counter(name, this);
children.put(name, counter);
}
return counter;
}

private void enter() {
CURRENT.set(this);
}

private void exit(float duration) {
synchronized (this) {
time += duration;
count++;
}
CURRENT.set(parent);
}

private static void indent(StringBuilder sb, int depth) {
for (int i = 0; i < depth; i++) {
sb.append(" ");
}
}

private synchronized void reset() {
children.clear();
count = 0;
time = 0;
}

private synchronized void write(StringBuilder sb, int depth) {
if (count > 0) {
indent(sb, depth);
if (parent.time != 0) {
sb.append((int)(100 * time / parent.time)).append("% ");
}

sb.append(name).append(" ")
.append("[total: ").append(time)
.append(" count: ").append(count)
.append(" average: ").append(time / count)
.append("]\n");
} else {
sb.append(name).append("\n");
}

if (children.size() > 0) {
List<Counter> sorted = new ArrayList<>();
sorted.addAll(children.values());
Collections.sort(sorted, new Comparator<Counter>() {
@Override
public int compare(Counter a, Counter b) {
return Float.compare(a.time, b.time);
}
});
for (Counter child : sorted) {
child.write(sb, depth+1);
}

float other = time;
for (Counter child : children.values()) {
other -= child.time;
}
if (other > 0) {
indent(sb, depth+1);
sb.append((int)(100 * other / time)).append("% <other>\n");
}
}
}

public static class Scope implements AutoCloseable {
private final long start;
private final Counter counter;

private Scope(Counter counter) {
this.start = System.nanoTime();
this.counter = counter;
counter.enter();
}

@Override
public void close() {
long end = System.nanoTime();
counter.exit((end - start) / 1000000000.0f);
}
}
}
Loading