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

Remove usage of custom marshaller #40169

Closed
10 of 12 tasks
alexdima opened this issue Dec 13, 2017 · 3 comments
Closed
10 of 12 tasks

Remove usage of custom marshaller #40169

alexdima opened this issue Dec 13, 2017 · 3 comments
Assignees
Labels
debt Code quality issues perf
Milestone

Comments

@alexdima
Copy link
Member

alexdima commented Dec 13, 2017

tl;dr - You shall not use URI in signatures or the types referenced from extHost.protocol.ts anymore. Use UriComponents and URI.revive instead

We use a custom marshaller in the extension host <-> renderer communication layer. It treats URI and RegExp specially, and makes them like JSON primitive types. This is helpful, for example, when passing an argument of type URI over the wire, you know for a fact that on the receiving side, the object is an instance of URI.

However, this is expensive, as it needs to run on each and every object sent over the wire, in a deep, recursive manner. It is very easy to avoid using the custom marshaller, simply replace the usage of URI with UriComponents which is JSON friendly (an interface without methods). Then, in the implementation, you can use URI.revive(...) to get back a real URI.

For RegExp objects (extremely rare usage), you're gonna have to roll your own strategy.

All edits should result in removing ProxyType.CustomMarshaller from the proxies defined in extHost.protocol.ts.

See b64355b for an example adoption


Remaining: 12 out of 49:

@alexdima alexdima self-assigned this Dec 13, 2017
@alexdima alexdima added the debt Code quality issues label Dec 13, 2017
@alexdima alexdima added the perf label Dec 13, 2017
@alexdima alexdima added this to the December 2017/January 2018 milestone Dec 13, 2017
jrieken added a commit that referenced this issue Dec 13, 2017
jrieken added a commit that referenced this issue Dec 13, 2017
jrieken added a commit that referenced this issue Dec 13, 2017
jrieken added a commit that referenced this issue Dec 13, 2017
jrieken added a commit that referenced this issue Dec 14, 2017
jrieken added a commit that referenced this issue Dec 14, 2017
jrieken added a commit that referenced this issue Dec 14, 2017
jrieken added a commit that referenced this issue Dec 14, 2017
@joaomoreno joaomoreno removed their assignment Dec 14, 2017
joaomoreno added a commit that referenced this issue Dec 14, 2017
jrieken added a commit that referenced this issue Dec 14, 2017
jrieken added a commit that referenced this issue Dec 14, 2017
@sandy081 sandy081 removed their assignment Dec 15, 2017
joaomoreno added a commit that referenced this issue Dec 15, 2017
jrieken added a commit that referenced this issue Dec 15, 2017
@alexdima alexdima self-assigned this Jan 24, 2018
@alexdima
Copy link
Member Author

fyi @dbaeumer I adopted it for tasks via fabd8a5

@alexdima
Copy link
Member Author

Done and support removed.

@dbaeumer
Copy link
Member

@alexandrudima thanks !

@vscodebot vscodebot bot locked and limited conversation to collaborators Mar 10, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
debt Code quality issues perf
Projects
None yet
Development

No branches or pull requests

6 participants