profiler
profiler copied to clipboard
WIP: Share resourceTable between threads
This isn't fully finished yet, but I wanted to put it up for discussion.
This makes two changes:
- It modifies the shape of the resourceTable to be array-of-structs instead of struct-of-arrays, and renames it to
resources. A resource is now typed as a tagged union and contains regular strings. We can afford to have resources as individual JS objects because the number of resources is usually quite low, and there's not a lot of GC pressure from those objects. - It moves the resource to the profile's root object, out of the thread. Resources are now shared between threads.
The more interesting aspect of this patch is the ripple effect it has on the rest of the profiler: Lots of places that were passing the thread around now need to pass around the thread and the resources, in separate function arguments or separate component props.
As we share more and more stuff, we'll need to pass more and more stuff in addition to the thread. This is going to get unwieldy.
I noticed that many places where I had to add an extra resources argument (or prop) also pass pages or categories (or even both).
I think it would make sense to group shared data into a profile.shared object. Then we can just pass the pair shared, thread in all these places, and we won't need to add more arguments as we move more things from thread to shared.
For example, in the beginning, we could have:
type Profile = {|
meta: ProfileMeta,
shared: ProfileSharedData,
threads: Thread[],
|};
type ProfileSharedData = {|
categories: CategoryList,
extensions?: ExtensionTable,
pages?: PageList,
libs: Lib[],
counters?: Counter[],
resources: Resource[],
|}
Thoughts? @julienw, @canova
I haven't really thought about what happens once we have a shared stackTable and need to apply per-thread transforms or search filtering. Would the return value of getFilteredThread be a { thread, shared } object? I'm not sure.
Edit: Hmm, I suppose there are two kinds of shared data: Data that is completely static, and data that is expected to change from thread filtering. Those things should probably be put into different objects, so that the UI doesn't suddenly have to deal with categories or pages that change depending on the thread filter.
I think it would make sense to group shared data into a
profile.sharedobject. Then we can just pass the pairshared, threadin all these places, and we won't need to add more arguments as we move more things fromthreadtoshared.
And why don't you want to pass the full profile object along with the thread object we want to proces?
Edit: Hmm, I suppose there are two kinds of shared data: Data that is completely static, and data that is expected to change from thread filtering. Those things should probably be put into different objects, so that the UI doesn't suddenly have to deal with categories or pages that change depending on the thread filter.
I guess I wouldn't put in a "shared" space things that might change (except for sanitization that generates a full profile anyway).
I think it would make sense to group shared data into a
profile.sharedobject. Then we can just pass the pairshared, threadin all these places, and we won't need to add more arguments as we move more things fromthreadtoshared.And why don't you want to pass the full profile object along with the thread object we want to proces?
Good question! Anything that wants to have a thread passed to it is usually only interested in that specific thread. So it feels weird to pass the full profile, which also contains all the other threads. The shared object would not contain any other threads. I was also thinking about memoization - there could be scenarios where the profile changes but a specific filteredThread is unaffected, and you wouldn't want to invalidate any cached data for that thread. But I think the cases where we permanently change data inside the profile are quite rare (really just symbolication, I think), so maybe this wouldn't be a problem.
Edit: Hmm, I suppose there are two kinds of shared data: Data that is completely static, and data that is expected to change from thread filtering. Those things should probably be put into different objects, so that the UI doesn't suddenly have to deal with categories or pages that change depending on the thread filter.
I guess I wouldn't put in a "shared" space things that might change (except for sanitization that generates a full profile anyway).
So you'd keep those as separate root properties?
Another solution I thought of would be to have a DerivedThread type which is not stored in the profile and which is only used by derived state. It could have both the per-thread properties and the shared properties on it. For example, if stackTable is shared, DerivedThread would still have a stackTable property even though Thread does not. And that stackTable property would usually just be the shared stackTable object. Only if a transform is applied to the thread, the filteredThread's stackTable property would be set to a post-transform stackTable.