From 553c9d74c84a3189a342bb3b6d8bdcfece885ae8 Mon Sep 17 00:00:00 2001 From: Ben Clayton Date: Mon, 11 Sep 2017 11:11:38 +0100 Subject: [PATCH] Prioritize contexts based on draws / frames. Fixes: #1058 --- .../com/google/gapid/models/ApiContext.java | 23 +++++-- gapis/api/gles/CMakeFiles.cmake | 1 + gapis/api/gles/importance.go | 43 ++++++++++++ gapis/api/gvr/CMakeFiles.cmake | 1 + gapis/api/gvr/importance.go | 23 +++++++ gapis/resolve/contexts.go | 66 +++++++++++++------ gapis/resolve/resolvables.proto | 1 + gapis/resolve/service.go | 2 +- gapis/service/service.proto | 2 + 9 files changed, 137 insertions(+), 25 deletions(-) create mode 100644 gapis/api/gles/importance.go create mode 100644 gapis/api/gvr/importance.go diff --git a/gapic/src/main/com/google/gapid/models/ApiContext.java b/gapic/src/main/com/google/gapid/models/ApiContext.java index 60cbf448fb..e9dfadfdc8 100644 --- a/gapic/src/main/com/google/gapid/models/ApiContext.java +++ b/gapic/src/main/com/google/gapid/models/ApiContext.java @@ -16,6 +16,7 @@ package com.google.gapid.models; import static java.util.Arrays.stream; +import static java.util.Comparator.comparingInt; import com.google.common.collect.Lists; import com.google.common.util.concurrent.Futures; @@ -38,7 +39,7 @@ public class ApiContext extends CaptureDependentModel { private static final Logger LOG = Logger.getLogger(ApiContext.class.getName()); - private FilteringContext selectedContext = FilteringContext.ALL; + private FilteringContext selectedContext = null; public ApiContext(Shell shell, Client client, Capture capture) { super(LOG, shell, client, Listener.class, capture); @@ -48,7 +49,7 @@ public ApiContext(Shell shell, Client client, Capture capture) { protected void reset(boolean maintainState) { super.reset(maintainState); if (!maintainState) { - selectedContext = FilteringContext.ALL; + selectedContext = null; } } @@ -96,21 +97,29 @@ protected void fireLoadStartEvent() { protected void fireLoadedEvent() { if (count() == 1) { selectedContext = getData()[0]; - } else if (selectedContext != FilteringContext.ALL) { + } else if (selectedContext != null) { selectedContext = stream(getData()) .filter(c -> c.equals(selectedContext)) .findFirst() - .orElse(FilteringContext.ALL); + .orElseGet(this::highestPriorityContext); + } else { + selectedContext = highestPriorityContext(); } listeners.fire().onContextsLoaded(); } + private FilteringContext highestPriorityContext() { + return stream(getData()) + .max(comparingInt(FilteringContext::getPriority)) + .orElse(FilteringContext.ALL); + } + public int count() { return isLoaded() ? getData().length : 0; } public FilteringContext getSelectedContext() { - return selectedContext; + return selectedContext != null ? selectedContext : highestPriorityContext(); } public void selectContext(FilteringContext context) { @@ -205,6 +214,10 @@ public Path.CommandTree.Builder commandTree(Path.CommandTree.Builder path) { .setAllowIncompleteFrame(true); } + public int getPriority() { + return context != null ? context.getPriority() : 0; + } + public Path.Events.Builder events(Path.Events.Builder path) { path.getFilterBuilder().setContext(id); return path; diff --git a/gapis/api/gles/CMakeFiles.cmake b/gapis/api/gles/CMakeFiles.cmake index 40c7ff8354..ba53a5da42 100644 --- a/gapis/api/gles/CMakeFiles.cmake +++ b/gapis/api/gles/CMakeFiles.cmake @@ -42,6 +42,7 @@ set(files guess_semantics.go helpers.go image.go + importance.go issue_whitelist.go links.go markers.go diff --git a/gapis/api/gles/importance.go b/gapis/api/gles/importance.go new file mode 100644 index 0000000000..6ef3c5300e --- /dev/null +++ b/gapis/api/gles/importance.go @@ -0,0 +1,43 @@ +// 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 gles + +import "github.com/google/gapid/gapis/resolve" + +var _ = []resolve.Importance{ + &GlDrawArrays{}, + &GlDrawArraysIndirect{}, + &GlDrawArraysInstanced{}, + &GlDrawElements{}, + &GlDrawElementsBaseVertex{}, + &GlDrawElementsIndirect{}, + &GlDrawElementsInstanced{}, + &GlDrawElementsInstancedBaseVertex{}, + &GlDrawRangeElements{}, + &GlDrawRangeElementsBaseVertex{}, + &EglSwapBuffers{}, +} + +func (*GlDrawArrays) Importance() int { return 1 } +func (*GlDrawArraysIndirect) Importance() int { return 1 } +func (*GlDrawArraysInstanced) Importance() int { return 1 } +func (*GlDrawElements) Importance() int { return 1 } +func (*GlDrawElementsBaseVertex) Importance() int { return 1 } +func (*GlDrawElementsIndirect) Importance() int { return 1 } +func (*GlDrawElementsInstanced) Importance() int { return 1 } +func (*GlDrawElementsInstancedBaseVertex) Importance() int { return 1 } +func (*GlDrawRangeElements) Importance() int { return 1 } +func (*GlDrawRangeElementsBaseVertex) Importance() int { return 1 } +func (*EglSwapBuffers) Importance() int { return 5 } diff --git a/gapis/api/gvr/CMakeFiles.cmake b/gapis/api/gvr/CMakeFiles.cmake index 013840f559..f791f02b84 100644 --- a/gapis/api/gvr/CMakeFiles.cmake +++ b/gapis/api/gvr/CMakeFiles.cmake @@ -25,6 +25,7 @@ set(files framebindings.go gvr.api gvr.go + importance.go mutate.go resolvables.proto types.api diff --git a/gapis/api/gvr/importance.go b/gapis/api/gvr/importance.go new file mode 100644 index 0000000000..394729ddbf --- /dev/null +++ b/gapis/api/gvr/importance.go @@ -0,0 +1,23 @@ +// 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 gvr + +import "github.com/google/gapid/gapis/resolve" + +var _ = []resolve.Importance{ + &Gvr_frame_submit{}, +} + +func (*Gvr_frame_submit) Importance() int { return 100 } diff --git a/gapis/resolve/contexts.go b/gapis/resolve/contexts.go index 4f74d438dc..5d0b08b6f6 100644 --- a/gapis/resolve/contexts.go +++ b/gapis/resolve/contexts.go @@ -16,6 +16,7 @@ package resolve import ( "context" + "sort" "github.com/google/gapid/core/data/id" "github.com/google/gapid/gapis/api" @@ -47,6 +48,12 @@ func Context(ctx context.Context, p *path.Context) (*InternalContext, error) { return boxed.(*InternalContext), nil } +// Importance is the interface implemeneted by commands that provide an +// "importance score". This value is used to prioritize contexts. +type Importance interface { + Importance() int +} + // Resolve implements the database.Resolver interface. func (r *ContextListResolvable) Resolve(ctx context.Context) (interface{}, error) { ctx = capture.Put(ctx, r.Capture) @@ -56,8 +63,8 @@ func (r *ContextListResolvable) Resolve(ctx context.Context) (interface{}, error return nil, err } - seen := map[api.ContextID]int{} - contexts := []*path.Context{} + priorities := map[api.ContextID]int{} + contexts := []api.Context{} s := c.NewState() err = api.ForeachCmd(ctx, c.Commands, func(ctx context.Context, i api.CmdID, cmd api.Cmd) error { @@ -67,22 +74,21 @@ func (r *ContextListResolvable) Resolve(ctx context.Context) (interface{}, error if api == nil { return nil } - if context := api.Context(s, cmd.Thread()); context != nil { - ctxID := context.ID() - idx, ok := seen[ctxID] - if !ok { - idx = len(contexts) - seen[ctxID] = idx - id, err := database.Store(ctx, &InternalContext{ - Id: string(ctxID[:]), - Api: &path.API{Id: path.NewID(id.ID(api.ID()))}, - Name: context.Name(), - }) - if err != nil { - return err - } - contexts = append(contexts, r.Capture.Context(path.NewID(id))) - } + + context := api.Context(s, cmd.Thread()) + if context == nil { + return nil + } + + id := context.ID() + p, ok := priorities[id] + if !ok { + priorities[id] = p + contexts = append(contexts, context) + } + if i, ok := cmd.(Importance); ok { + p += i.Importance() + priorities[id] = p } return nil }) @@ -90,5 +96,27 @@ func (r *ContextListResolvable) Resolve(ctx context.Context) (interface{}, error return nil, err } - return &service.Contexts{List: contexts}, nil + sort.Slice(contexts, func(i, j int) bool { + return priorities[contexts[i].ID()] < priorities[contexts[j].ID()] + }) + + out := &service.Contexts{ + List: make([]*path.Context, len(contexts)), + } + + for i, c := range contexts { + api := c.API() + ctxID := c.ID() + id, err := database.Store(ctx, &InternalContext{ + Id: string(ctxID[:]), + Api: &path.API{Id: path.NewID(id.ID(api.ID()))}, + Name: c.Name(), + Priority: uint32(i), + }) + if err != nil { + return nil, err + } + out.List[i] = r.Capture.Context(path.NewID(id)) + } + return out, nil } diff --git a/gapis/resolve/resolvables.proto b/gapis/resolve/resolvables.proto index 5f6385b261..b05ce78eaa 100644 --- a/gapis/resolve/resolvables.proto +++ b/gapis/resolve/resolvables.proto @@ -29,6 +29,7 @@ message InternalContext { string id = 1; path.API api = 2; string name = 3; + uint32 priority = 4; } message CommandTreeResolvable { diff --git a/gapis/resolve/service.go b/gapis/resolve/service.go index a1c8c1cab8..1eec847b30 100644 --- a/gapis/resolve/service.go +++ b/gapis/resolve/service.go @@ -25,7 +25,7 @@ func internalToService(v interface{}) (interface{}, error) { case api.Cmd: return api.CmdToService(v) case *InternalContext: - return &service.Context{Name: v.Name, Api: v.Api}, nil + return &service.Context{Name: v.Name, Api: v.Api, Priority: v.Priority}, nil default: return v, nil } diff --git a/gapis/service/service.proto b/gapis/service/service.proto index 2aad15a741..1c14685d5a 100644 --- a/gapis/service/service.proto +++ b/gapis/service/service.proto @@ -619,6 +619,8 @@ message Context { string name = 1; // The API that this context belongs to. path.API api = 2; + // The estimated importance for the context. 0 = lowest priority. + uint32 priority = 3; } // CommandTree represents a command tree hierarchy.