Skip to content
This repository was archived by the owner on Aug 21, 2020. It is now read-only.

Large binary data generates a file that takes too long to compile. #8

Closed
pekim opened this issue Jun 28, 2017 · 4 comments
Closed

Large binary data generates a file that takes too long to compile. #8

pekim opened this issue Jun 28, 2017 · 4 comments
Assignees
Labels

Comments

@pekim
Copy link

pekim commented Jun 28, 2017

I have a number of truetype files that result in a generated go src file of about 150,000 lines.

Each input file generates something like this.

var _bindata = map[string]*asset{
	"DejaVuSans-Bold.ttf": &asset{
		name: "DejaVuSans-Bold.ttf",
		data: "" +
			"\xec\xfd\x0b\x7c\x15\xd5\xd5\x37\x8e\x7f\xd7\xde\x33\x73\xee\xf7\x49\xe6\x08\x21\x39\x40\x0c\x10" +
			"\xb9\x89\x88\x88\x88\x48\x23\x22\x02\x62\x88\x80\x48\x81\x70\x33\xdc\x22\x86\x48\x11\x91\x87\x52" +
			"\xaa\xc8\x83\x34\x22\x5a\x8a\x88\x14\x29\x20\x45\x8a\x3c\x14\x11\xa9\x22\x22\x45\x8b\x88\x48\x29" +
			// ... about another 15,000 lines ...
			"\x24\x64\x88\xcf\x90\x8a\xed\x24\x70\x42\xfb\x36\xe5\x56\xe5\x16\x65\xb0\x38\x32\x58\x11\x07\x09" +
			"\x6d\x3b\x86\x32\x83\x2c\x23\x83\x14\xab\x26\x21\x96\xa1\x26\x81\x75\xd2\x2a\x6f\xd9\x6e\x35\xde" +
			"\x1c\x6a\x1a\x6f\x5e\x0d\x35\xff\x07\x00\x00\xff\xff",
		size: 705684,
	},

The large number of lines of concatenated strings results in crazy compile times. In fact I've not been able to successfully build; I end up giving up and interrupting the build.

If I change wrapAt to a very large number (such as 1000000) then the data is generated as a single string for each asset, and the build time is perfectly reasonable.

I can't find a nice way to change, or better still disable, the wrapAt value. For now I've locally changed
{{flate . "\t\t\t" 24}}
to
{{flate . "\t\t\t" 1000000}}
at release.go#L213
to prove the problem and a solution.
(And the same for {{read . "\t\t\t" 24 -}}.)

It would be good if this could configured in https://godoc.org/github.com/tmthrgd/go-bindata#GenerateOptions . However I've not yet been able to get my head around the code enough to work out how to approach this, and make a merge request.

Perhaps the concatenation of multiple strings should be removed completely? I'm sure that others may hit the same issue at some point. Interestingly the original jteeuwen/go-bindata does not do this, it generates a single string for an asset's data.

pekim added a commit to pekim/go-bindata that referenced this issue Jun 28, 2017
Do not generate data with a new string every 24 bytes, but with
strings of up to 1,000,000 bytes. This is a horrible hack; a better
approach needs to be found.

The reason behind this change is that compiling a concatenation of
many thousands of string literals is extremely slow.

see also tmthrgd#8
@tmthrgd tmthrgd self-assigned this Jun 29, 2017
@tmthrgd tmthrgd added the bug label Jun 29, 2017
@tmthrgd
Copy link
Owner

tmthrgd commented Jun 29, 2017

I likely won't have a chance to look at this for the next few days.

I'm almost certain this is related to golang/go#16394 which is about build failures caused by a large number of string concatenations. While that remains open, I'll try the workaround applied to x/text. Iff that doesn't work, I'll add a configuration option that will allow the disabling of string wrapping.

I'd rather not add a configuration option if I can help it and it would later be removed if the compiler bug is fixed.

@pekim
Copy link
Author

pekim commented Jun 29, 2017

I agree, golang/go#16394 does indeed look like it describes the problem.

The parenthesis workaround sounds good if it works.

I'd rather not add a configuration option if I can help it and it would later be removed if the compiler bug is fixed.

Yes, on reflection that would result in a backwards incompatible API change. Never a good idea if it can be avoided.

@tmthrgd tmthrgd closed this as completed in 0def143 Jul 2, 2017
tmthrgd added a commit that referenced this issue Jul 2, 2017
@tmthrgd
Copy link
Owner

tmthrgd commented Jul 2, 2017

@pekim I was able to trivially reproduce this bug, and porting the fix from x/text solved it for me. Hopefully this is also a satisfactory solution for you.

@pekim
Copy link
Author

pekim commented Jul 2, 2017

Yes, it works a treat.

Thank you.

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

No branches or pull requests

2 participants