Fix bigint support in messages sent between webviews and vscode #54
Conversation
bhufmann
force-pushed
the
fix-serialization
branch
from
309587c to
549ee29
Compare
Signed-off-by: Bernd Hufmann <[email protected]>
bhufmann
force-pushed
the
fix-serialization
branch
from
549ee29 to
15af650
Compare
|
@ngondalia this pull request upgrades the vscode-trace-extension to the latest traceviewer code and corresponding Trace Compass trace server. It has some shortcuts to make it work. But I think it's good step forward. Any objections me merging it? |
We are using a custom trace server, so there are some issues I am running into with these changes. Everything works fine the first time i open the trace, but once i close it and re-open it, there is bunch of weird behavior where the views wont load and everything disappears.. seems like a problem on our server because it doesn't seem to happen on tracecompass server. Any ideas on what it could be? Do i need to make any changes on server-side for this fix? |
|
Not sure. But I have a suspicion. We changed the API on how the data samples are requested from the backend. Instead of providing a list of timestamps, now a range with start, end and number of samples is used. This was to avoid send large arrays in fetch methods. See [1] for the change in the TSP and [2] for the front-end change. See also [3] and [4] for the corresponding Trace Compass server change. If your custom trace server is not able to handle that the new parameters it would explain some problems with the visualization. You will have to update your server for that. [1] eclipse-cdt-cloud/trace-server-protocol#82 |
|
@ngondalia I'm going to merge. The updated version has more features and performance improvements, especially in the timegraphs. I hope making your server work with it is easy to do. Please let me know if you have any questions. |
Yeah, no problem. I am still looking into it why its not working. I have traced the exception down to onOverviewRemoved() in vscode-trace-viewer-container.tsx. For some reason,' this.state' is undefined when this callback occurs and this causes everything to go in a weird state. Still investigating as to why this is occuring. Are there any changes that needs to be made with custom trace types with the introduction of overview view? Still fairly new to this codebase, so not sure how a lot of the things work. |
Ahh, good hint about |
TODO: JSONBig.parse() doesn't create bigint if numbers are small. This has to be addressed when deserializing messages. Re-use the normalizer of the tsp-typescript-client to do that. The following branch in the tps-typescript-client is introducing an utility class and, if accepted, it can be used to solve the deserialization issue: eclipse-cdt-cloud/tsp-typescript-client#56 Contributes to fixing eclipse-cdt-cloud#35 Signed-off-by: Bernd Hufmann <[email protected]>
bhufmann
force-pushed
the
fix-serialization
branch
from
15af650 to
2da54f7
Compare
With this PR, the overview view is shown only using the Trace Compass trace server. The data provider is hard-coded to a specific data provider. If this data provider doesn't exist on the back-end (e.g. your back-end) then it is not shown. To support it properly more work is needed. I opened a tracker for that #55. It's also not possible to open the overview after closing it (trace needs to re-opened) for that nor configure the default overview data provider. The idea of the overview view is that it shows the whole trace for a given XY data provider (only XY at the moment) and users can see where to zoom in, and where the zoom window range is etc. |
TODO: JSONBig.parse() doesn't create bigint if numbers are small. This has to be addressed when deserializing messages. Re-use the normalizer of the tsp-typescript-client to do that.
The following branch in the tps-typescript-client is introducing an utility class and, if accepted, it can be used to solve the deserialization issue:
eclipse-cdt-cloud/tsp-typescript-client#56
Contributes to fixing #35
Signed-off-by: Bernd Hufmann [email protected]