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
encoding/json: remove fmt dependency? #15583
Comments
fmt
from encoding/json
?
Maybe? Got any before/after numbers? /cc @crawshaw |
None yet; you mean for binary size or performance-in-error? Contributing to Go is something of a time investment so I didn't bother doing any work before checking if there was interest. |
For whatever metric you cared enough about to file this bug. |
Ha, OK: background then. I'm using GopherJS to transpile some projects to JS for a dashboard. While this is the sort of case where large JS files are OK, because it's a long-running browser App and load-times are OK, I noticed that the binaries were huge. Two obvious culprits were So, while Go binaries being large is generally simply taken as a fact of life in ..hence my recent crusade to try and reduce the use of |
So try it and see how much smaller the Javascript gets. |
How many applications will benefit from this?
I imagine it'd be find an application that only uses encoding/json but not
fmt or any other packages that use fmt.
|
Is there not a mild intrinsic value in partitioning, anyway? And value also in using the appropriate tool, perhaps in this case As I said, formatting of On 7 May 2016 00:23:24 IST, Minux Ma notifications@github.com wrote:
Sent from my Android device with K-9 Mail. Please excuse my brevity. |
I removed the dependency on fmt from encoding/json by using strconv.Quote.
(quick and dirty patch at
https://gist.github.com/minux/f9b68a62be8c5983b70ffc756824142f)
Then compiling a simple test program that only json.Marshal a simple object:
Before: 2014143 bytes
After: 1832613 bytes
There are 9% reduction on binary size. But again, I don't think removing
the fmt dependency would benefit many real applications (not to mention
that removing fmt means making the code in encoding/json less readable.)
|
The other benefit to fewer dependencies is that the dependency graph is sparser, so more things can be compiled concurrently. This applies whether or not the code uses I think the patch could be made nicer by introducing local functions |
@josharian: Interesting, I hadn't even considered that; still a lot of compiler-fu to learn. :) @minux: I don't think that harms readability much, personally? Also, 9% is pretty significant in my book, and may work out even better for downstream hax like GopherJS. For clarity, a variadic local errorf as @josharian suggests would clean things up significantly. |
@minux: I made some changes to some GopherJS code to avoid Addendum; these successful reductions also came from removing |
The huge reduction probably suggests there is something
very inefficient about the fmt package. I'd suggest figure
out why GopherJS compiler generates so much Javascript
for the fmt package first.
The 9% reduction only applies to toy examples. Again, I
can't think of any real world Go application that only uses
encoding/json, but not (directly or indirectly) fmt.
|
Plenty of my packages use JSON but not fmt? Not all applications push out human-readable text output yunno. ;) On 8 May 2016 20:55:52 IST, Minux Ma notifications@github.com wrote:
Sent from my Android device with K-9 Mail. Please excuse my brevity. |
fmt is not only about user output. Of the 155 std packages, 83 of them uses fmt. Basically, the only way such a program can interact |
It's no surprise that text-heavy tools use fmt, but a lot of packages use it only for Errorf. If the argument is that fmt is unavoidable, and we should learn to love to bomb.. I don't buy it. If they're all like I've just stripped On 8 May 2016 23:33:31 IST, Minux Ma notifications@github.com wrote:
Sent from my Android device with K-9 Mail. Please excuse my brevity. |
@minux - I think you're right that it would be rare for |
I agree with @minux that almost all real programs will bring in fmt anyway and you should first understand why fmt compiles to such large GopherJS output code first. The other possibility is modify the GopherJS compiler to replace fmt.Errorf with a lighter work-alike. But that's kinda fixing the wrong problem. I don't want to start making every package depending on fmt have its own fmt-like "minimal" versions. That would mean that in a real program we'd have both the actual fmt implementation as well as all the "minimal" fmt clones, making the size problem even worse. I'd rather just see the linkers (Go and GopherJS's) fixed to do more dead code elimination, if that's the issue. (sometimes unnecessary generated init funcs pin things that might not otherwise be needed). But if it turns out that a perfect linker couldn't dead-code eliminate something (perhaps because the code paths to take depend on the Errorf format string), perhaps only then could we change something to give the linker more information. But only if it helps significantly. |
Personally I feel that, architecturally, As it happens, making "lite" versions for my GopherJS projects is exactly what I'm doing right now. I'm making a Perhaps the most valuable change would be to make |
A good linker should discard things which are unused. I don't know whether GopherJS can do that. |
None of the Scanf code is included in a binary if you use only the print functions in fmt. |
I agree with @bradfitz here. I think that trying to remove a single dependency from a package in order to optimize performance is setting a bad precedent. The Go programming language and its compilers are meant to make our jobs, the humans, easier, and I want to continue to be be able to write natural and idiomatic Go code.
This is where I'd like to see time and effort being spent, rather than trying to eliminate usages of packages in a few places, because that is likely to regress in the future, or cause pain for people trying to write idiomatic Go code because suddenly there are certain packages they shouldn't import (according to fuzzy metrics).
GopherJS is able to and currently performs high quality DCE, potentially better than generic JS minifiers can, because it has full access to the original source that's being built and it sees what is statically needed/used. It follows the constraint that it must always produce valid code that works as specified in Go spec; it can't take shortcuts that would be against Go spec. However, its DCE efforts are significantly thwarted because of run-time reflection via There is an open issue gopherjs/gopherjs#186 to implement optional "unsafe aggressive DCE" where if you promise not to use reflect to access things that aren't statically mentioned, GopherJS could eliminate them (and if you break your promise, behavior is undefined). However, it's not implemented yet. |
The GopherJS compiler has access to the full source code, so it can tell if
a certain type has not been used at all (even with reflect, you still can't
use a type that has not been created statically somewhere in the program.)
Then it should be able to eliminate the scanner related types (and methods)
in the fmt package. See also recent @crawshaw's changes on linker DCE and
reflect.
|
I think that's true for types, but I don't think it is for methods. Sorry, I should've been more precise in my post above (instead of saying "types, etc."). See https://godoc.org/reflect#Value.MethodByName. An arbitrary program can get user input from keyboard or over network as a string and use Not being able to eliminate methods causes a chain reaction, since those methods will mention other funcs, types and their methods, and usually pull in large parts of Go packages that are not statically used.
Those are helpful, thanks for pointing them out! For reference, what I found is what's mentioned at bottom of #6853 and https://github.com/golang/go/commits/master?author=crawshaw. |
That's not what is happening here. The gc toolchain does eliminate all the
fmt scanning functions from a simple example program that uses
encoding/json.
Even if you can obtain a non-exported method by name, you still can't do
much on it, so an ideal linker could still remove the unreferenced method
body and only leave a placeholder. Again, similar schemes have been
implemented in gc, search crawshaw's recent changes for details.
|
No thanks. I think the expectation is that basically every Go program in the world is going to import fmt one way or the other. |
Would there be interest in a PR that removes
fmt
as a dependency inencoding/json
, if doing so didn't overly complicate the code?I ask because
fmt
is a large dependency to pull in for what appears to be exclusively error formatting indecode.go
.Happy to contribute this.
The text was updated successfully, but these errors were encountered: