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

Load JSON internally by default #299

Merged
merged 1 commit into from Oct 23, 2020
Merged

Load JSON internally by default #299

merged 1 commit into from Oct 23, 2020

Conversation

ghost
Copy link

@ghost ghost commented Oct 10, 2020

In runtime, v2ray calls v2ctl ONLY to load json file.
V2ray should work perfectly fine when json is loaded internally, that way v2ctl is NOT needed to run it.
V2ctl is still provided so no feature is missing. Binary size remains unchanged.
There is no good reason not to do it.

@kslr
Copy link
Contributor

kslr commented Oct 13, 2020

这能做什么?

@ghost
Copy link
Author

ghost commented Oct 13, 2020

v2ray在运行时唯一调用v2ctl的地方就是在读取JSON文件的时候。这包括单文件和多文件。

buf, err := ctlcmd.Run(append([]string{"config"}, files...), os.Stdin)

这个更改让v2ray可以直接读取JSON文件,不依赖v2ctl。但是只能读取单个文件,所以撤销。
如果多文件这个功能移植到v2ray里,就可以脱离v2ctl,而v2ctl继续提供。
v2ctl的其他功能保持不变,不需要合并到v2ray里,因为不是运行时必要的。

@ghost ghost closed this Oct 13, 2020
@ghost ghost reopened this Oct 23, 2020
@xiaokangwang
Copy link
Contributor

"Binary size remains unchanged." is inaccurate.
This change will increase binary size by 400~500 kb, which isn't that big since V2 is already 17MB. However, it should be noted that even if json can be loaded internally, the GEO files might still be needed, which is about 6-7 MB.

@xiaokangwang
Copy link
Contributor

The reason you find binary size is unchanged is because of a bug in mutijson, which have been fixed in #351 .

@ghost
Copy link
Author

ghost commented Oct 23, 2020

@xiaokangwang In low-end boxes where free space is limited, iptables is used to rule out most of the traffic that should go directly for better performance, that way geoip.dat is not needed. It's exactly in this kind of use case that people try to reduce binary size. Even if the binary size may increase, getting rid of v2ctl is a big deal.

@xiaokangwang
Copy link
Contributor

Yes, I agree with you on that. In the most of case, 400-500 kb of space isn't a deal breaker anyway. V2Fly team have already discussed on this and decided to use jsonem by default.

@xiaokangwang xiaokangwang self-requested a review October 23, 2020 15:58
@kslr kslr merged commit 0dc1764 into v2fly:master Oct 23, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants