-
Notifications
You must be signed in to change notification settings - Fork 40k
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
Provide an int64 version of Quantity that is much faster #25243
Conversation
bbee647
to
63bac7e
Compare
63bac7e
to
d67c965
Compare
ab12e56
to
7e6a27e
Compare
7e6a27e
to
964ec06
Compare
964ec06
to
6a0789e
Compare
Naw, not going to squash. |
Spawned #25886 for reenabling powerpc |
Bumping since this has the powerpc disablement and blocks proto. |
@k8s-bot test this [submit-queue is verifying that this PR is safe to merge] |
GCE e2e build/test passed for commit 2c9b83f. |
Manual merge. |
@luxas, first of all let me take this opportunity to sincerely thank you for the work you did to add the PowerPC arch support. Kubernetes on PowerPC is very important for IBM and we are ramping up the effort as well. Looking forward to your continued support and guidance. We are looking into the linker issue and will keep the community posted. |
Ref #8132 |
I just wanted to publicly acknowledge the horror that is circular vendor dependencies... I recognize there was no way to make this change without manually modifying the vendored appc package, but this now adds a bunch of etxra steps to updating Godeps. Can we move the resource package to its own repo? |
We can. On Fri, May 20, 2016 at 2:41 PM, Tim St. Clair notifications@github.com
|
I'm looking into the PPC64 linker error. As mentioned above, the problem occurs when building the hyperkube binary. The intermediate go.o file that is generated contains a single text segment that is too large (>2**24 bytes). An interesting question was brought up above that relates to this problem. This file seems to be pulling in many, many packages. Has anyone investigated whether or not they are all really needed? Unfortunately you are hitting this error on ppc64le but it indicates that even when the programs are successfully built maybe they are larger than they need to be. Also, as new code gets developed the programs will continue to grow in size and you will probably at some point hit linker overflow errors like this on other platforms. |
Kube is probably one of the largest Go ecosystem projects (juju is larger, Honestly, I expect us to grow to at least 2x our current package set size On Wed, May 25, 2016 at 10:23 AM, Lynn Boger notifications@github.com
|
The plan is to fix the problem so that the GNU linker can handle the build of a golang program like this. I was just wondering if the size of the program was a concern and if there were other ways to reduce the size. |
It's not a concern per se - some folks have even made statements like On Wed, May 25, 2016 at 12:48 PM, Lynn Boger notifications@github.com
|
Hey. This is wrong. This is totally and utterly wrong. It makes our vendored code not reproducible. I can no longer godep restore/save and get my original values back. This has to roll back. |
I believe upstream has fixed their circular dependency on us now, so we can On Fri, May 27, 2016 at 6:22 PM, Tim Hockin notifications@github.com
|
Correction - updated their circular dependency. On Fri, May 27, 2016 at 6:24 PM, Clayton Coleman ccoleman@redhat.com
|
appc 0.8.4 has the change. On Fri, May 27, 2016 at 6:25 PM, Clayton Coleman ccoleman@redhat.com
|
no, upstream for github.com/appc/spec/schema/types/isolator_resources.go is not fixed - ParseQuantity returns a value now, and they need a pointer |
ab50d12e88f57788bf84b83fef2be236eb1fcc0b has it. The latter reverted? On Fri, May 27, 2016 at 6:29 PM, Tim Hockin notifications@github.com
|
7da0fdd4dfce6fda81869ac6ecd6ef42247957ea undid it |
I'm going to bump to #26459 On Fri, May 27, 2016 at 6:42 PM, Tim Hockin notifications@github.com
|
Opened #26463 to verify that verify-godeps.sh is doing its job - pretty sure it was returning green on this PR the entire time, which is why we didn't fix this before it merged. |
Resolves #25019 by adding an int64 version of Quantity that is aware of overflow and can spill to the inf.Dec version. Heavily optimized all the paths for allocation and efficiency.
Known issues
This change is