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

Fails to Get/Parse Valid JSON #239

Closed
nahtnam opened this issue Aug 17, 2018 · 12 comments
Closed

Fails to Get/Parse Valid JSON #239

nahtnam opened this issue Aug 17, 2018 · 12 comments
Labels

Comments

@nahtnam
Copy link

nahtnam commented Aug 17, 2018

  • VSCode Version: 1.26.1
  • OS Version: 10.13.5
  • REST Client Version: 0.19.1

Steps to Reproduce:

  1. Make a test.http file.
  2. Add the following
GET https://gist.githubusercontent.com/nahtnam/cd842afc31d90c3417c060e926fca656/raw/dc9f6d41a4613e7c56d4cc8458719b0b6130cf39/sample.json
  1. Run it

It will get stuck and not show anything and eventually slow down the rest of vscode as well.

@Huachao
Copy link
Owner

Huachao commented Aug 18, 2018

@nahtnam it seems that I can view the response successfully except that it will cost a longer time.

@nahtnam
Copy link
Author

nahtnam commented Aug 20, 2018

Ah I see, it takes about 25 seconds to load, and even then it is un-prettified. Is there any way to increase the performance? The API returns within 300ms so it should really only take a few seconds max right?

@Huachao
Copy link
Owner

Huachao commented Aug 21, 2018

@nahtnam the root cause of this question is that the content-type of response body is not application/json, it's text/plain instead. In current logic, if it's not known MIME type, like json, xml and so on, I can't figure out what the language of the response to highlight. And I will use the auto highlight logic of the highlight library which costs a lot of time. So I have prepared a fix that when deciding the highlight language of response body, I will consider both content-type of response body and the content of the response body. So for your case, although the content type is not json, when I check the content of the response body, it's json actually, I will use the json language to highlight the response body, not the auto mechanism.

@Huachao
Copy link
Owner

Huachao commented Aug 21, 2018

@nahtnam after the update, it will take less than 10 seconds to load. I will keep improving the performance for rendering.

@Huachao Huachao closed this as completed Aug 21, 2018
@nahtnam
Copy link
Author

nahtnam commented Aug 21, 2018

Awesome, any idea when this feature will be released into the market?

Really appreciate the work done here, this library is very useful!

@Huachao
Copy link
Owner

Huachao commented Aug 22, 2018

@nahtnam I would like to publish a new release together with other features and bug fixes in early September.

@nahtnam
Copy link
Author

nahtnam commented Aug 22, 2018

Gotcha, awesome, thanks!

@nahtnam
Copy link
Author

nahtnam commented Aug 22, 2018

Actually @Huachao The issue is still there. Please try with this API URL: https://apituner.ecbsn.com/apituner/v1/store/reward/list?channel=4

On my computer, it takes forever and eventually ends up freezing VS Code. Is it because the JSON is big and VS Code cannot handle it or is it because you are processing the JSON?

@Huachao Huachao reopened this Aug 22, 2018
@nahtnam
Copy link
Author

nahtnam commented Aug 22, 2018

I don't completely know how the whole system but could it be a possibility that js-beautify is the bottleneck? JSON.stringify({ hello: 'world' }, null, 2) is very very fast, is it possible to use that instead (if the only thing you use beautify is for making the JSON pretty)?

@Huachao
Copy link
Owner

Huachao commented Aug 23, 2018

@nahtnam the e2e latency for a user to view a complete response in the panel in my extension can be divided into three phases:

  1. Send the request and wait for the response (Spinner keep spinning in the status bar)
  2. Do some post-processing jobs for the response in my extension like Prettify the response body, Identify the links in response body, Calculate line number, Syntax highlight. (Spinner froze in the status bar)
  3. VSCode do the rendering HTML job (Spinner disappeared, replaced with duration and response size, and a response panel already created, however with no content)

And in my test from my local machine, I can see the first phase costs only <1s, the second phase costs <8s, and the last rendering phase costs <15s.

So can you calculate the duration for each phase to see which part costs the most? In my previous data, the bottleneck is from the third phase, not the second phase. And I will keep improving the performance of my extension in the second phase.

And back to your question, why do we need a third party library like js-beautify, can we use the built-in JSON object? I have to say no, the story is a bit more complicated. Firstly, we can't do the stringify directly, since what we have is the response body, it's a string, not an object. So if we would like to use it, we need deserialize first and serialize back. However, in the deserialize and serialize process, we may lose some information, consider the following example,

{"value": 1.0}

After the deserialization and serialization process, the result will be

{
   "value": 1
}

And the fraction part is lost. So we can't simply use this way although this is the fastest way in my perf test. So I have to use a prettify libray like js-beautify which is the fastest way in my test.

@Huachao
Copy link
Owner

Huachao commented Dec 26, 2018

@nahtnam I have improved the performance of the second phase (processing received response), the duration costs less than 2 seconds on my local machine for your request. I'd like to close it. I will publish the fix in next release.

@Huachao Huachao closed this as completed Dec 26, 2018
@Huachao
Copy link
Owner

Huachao commented Jan 3, 2019

@nahtnam you can try the latest version 0.21.0 to verify

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants