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

add more GC.stats #3405

Merged
merged 1 commit into from
Oct 9, 2016
Merged

add more GC.stats #3405

merged 1 commit into from
Oct 9, 2016

Conversation

kostya
Copy link
Contributor

@kostya kostya commented Oct 9, 2016

No description provided.

@asterite
Copy link
Member

asterite commented Oct 9, 2016

Thanks! I wanted to this a couple of times but always forgot to do it... :-)

@asterite asterite merged commit 0c93b1b into crystal-lang:master Oct 9, 2016
@RX14
Copy link
Member

RX14 commented Oct 12, 2016

This seems to have broken my build on arch linux:

G-C-.o: In function `*GC::stats:GC::Stats':
GC:(.text+0x419): undefined reference to `GC_bytes_found'
collect2: error: ld returned 1 exit status
Error: execution of command failed with code: 1: `cc -o "/data/programming/crystal/.build/std_spec" "${@}"  -rdynamic  -lyaml `/usr/bin/llvm-config --libs --system-libs --ldflags 2> /dev/null` -lstdc++ `command -v pkg-config > /dev/null && pkg-config --libs libssl || printf %s '-lssl -lcrypto'` `command -v pkg-config > /dev/null && pkg-config --libs libcrypto || printf %s '-lcrypto'` -lz -lreadline -lgmp -lxml2 -lpcre -lm -lgc -lpthread /data/programming/crystal/src/ext/libcrystal.a -levent -lrt -ldl -L/usr/lib -L/usr/local/lib`

It seems from the source as if GC_bytes_found is a private variable inside bdwgc, so i'm not sure if we should reply on it being present.

@RX14
Copy link
Member

RX14 commented Oct 12, 2016

Seems as if the core issue is that the arch package is built with --disable-static, which hides private symbols, of which GC_bytes_found is one. So I don't think it's appropriate to use.

@kostya
Copy link
Contributor Author

kostya commented Oct 12, 2016

bytes_found was before this commit, and also this is strange value not showing anything, i think need to delete it.

@RX14
Copy link
Member

RX14 commented Oct 12, 2016

@kostya but it didn't have a spec before this commit, so .stats was never called, and so the symbol didn't make it into the binary for the linker to error at.

@RX14
Copy link
Member

RX14 commented Oct 12, 2016

This is #837 again, but with a different symbol from LibGC.

@asterite
Copy link
Member

Don't worry, we'll eventually provide our own LibGC fork because we already made some changes to it to support multiple threads, so these functions will be available everywhere.

@RX14
Copy link
Member

RX14 commented Oct 12, 2016

@asterite how am I meant to contribute to crystal in the meantime? Revert the commit every time I need to run specs, and unrevert every time I need to push? The point is that we're using LibGC's internal API, and that's not portable.

@asterite
Copy link
Member

I see. We can probably remove stats then and someone can provide it as a temporary shard.

@asterite
Copy link
Member

@RX14 Let me know if it compiles now. I think GC_get_heap_usage_safe is exposed, and that's the only thing used now.

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.

None yet

3 participants