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

Fix compatibility with zeromq 4.1 #84

Merged
merged 1 commit into from
Jul 7, 2015
Merged

Fix compatibility with zeromq 4.1 #84

merged 1 commit into from
Jul 7, 2015

Conversation

yuyichao
Copy link
Contributor

@yuyichao yuyichao commented Jul 7, 2015

Fix #83.

The padding size matches what's in the zmq header now. (On 0.4-dev this can be a NTuple{8, Int64}...)

@tkelman
Copy link
Contributor

tkelman commented Jul 7, 2015

So did this change between different ZMQ versions? Any compatibility issues with distros that are not as bleeding-edge? I guess Travis will build 3.2.4 from source?

@stevengj
Copy link
Contributor

stevengj commented Jul 7, 2015

See also zeromq/libzmq@d9fb1d3 ... do we need to have an explicit Int64 field here to ensure alignment?

@stevengj
Copy link
Contributor

stevengj commented Jul 7, 2015

@tkelman, yes, apparently they expanded the size of the struct in a recent version. As long as we use a padding size that is ≥ any size used in any version of ZMQ, we should be fine, because we don't actually access any of that data ourselves.

@yuyichao
Copy link
Contributor Author

yuyichao commented Jul 7, 2015

do we need to have an explicit Int64 field here to ensure alignment?

I hoped that a big bitstype would do but I don't mind adding a Int64 at the beginning just to be safe.

Any compatibility issues with distros that are not as bleeding-edge?

Hopefully not. And this is what I want to check with the CI builds.

@yuyichao
Copy link
Contributor Author

yuyichao commented Jul 7, 2015

Is zeromq/libzmq@d9fb1d3 released yet? I don't see that on 4.1.2. Should we increase the size of the structure by 1 pointer then?

P.S. I think the GC allocation are aligned now, which should be all what matters? @carnaval

@stevengj
Copy link
Contributor

stevengj commented Jul 7, 2015

@yuyichao, that patch does not increase the size of zmq_msg_t — it changes the struct to a union in order to ensure alignment.

If our GC allocation is aligned, that is all that matters.

@yuyichao
Copy link
Contributor Author

yuyichao commented Jul 7, 2015

it changes the struct to a union in order to insure alignment.

Ah.... I think this is not the first time I missed sth like this .............. = = ....

@yuyichao
Copy link
Contributor Author

yuyichao commented Jul 7, 2015

To the best of my knowledge, a huge bitstype should garentee maximum alignment

https://github.com/JuliaLang/julia/blob/f4283920037399a0ecefa4507adbc6eb9a25708e/src/alloc.c#L654

@stevengj
Copy link
Contributor

stevengj commented Jul 7, 2015

Sound good. Let's merge and tag this once Travis is green.

@yuyichao
Copy link
Contributor Author

yuyichao commented Jul 7, 2015

I haven't checked the upstream tracker but at least I can confirm that the tests all pass and I have just plotted a curve in IJulia with PyPlot.

(Still find it wierd that I had a working IJulia on earlier master. Did we tighten the allocation recently?....)

yuyichao added a commit that referenced this pull request Jul 7, 2015
Fix compatibility with zeromq 4.1
@yuyichao yuyichao merged commit fab71ed into master Jul 7, 2015
@yuyichao yuyichao deleted the yyc/zmq-4.1 branch July 7, 2015 18:17
@yuyichao
Copy link
Contributor Author

yuyichao commented Jul 7, 2015

Should we tag a new release?

@stevengj
Copy link
Contributor

stevengj commented Jul 7, 2015

Yes, I'll tag it.

@yuyichao
Copy link
Contributor Author

yuyichao commented Jul 7, 2015

THX.

@stevengj
Copy link
Contributor

stevengj commented Jul 7, 2015

Tagged as v0.1.19.

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.

3 participants