C-Blocks icon indicating copy to clipboard operation
C-Blocks copied to clipboard

Partially split Blocks.xs into some .c/.h files?

Open tsee opened this issue 8 years ago • 11 comments

I've been wondering: Blocks.xs is pretty big at this point. How would you feel about splitting some of it out into their own .c/.h files to decouple things a bit (where possible)?

tsee avatar Jan 11 '17 15:01 tsee

Closing this because superseded by #20.

tsee avatar Jan 11 '17 16:01 tsee

Reopening until I finish extracting the parser code.

run4flat avatar Jan 12 '17 12:01 run4flat

Quoting your concern in the pull request.

Something I've always struggled with: do the compiled object files get linked to all compiled xs files? In other words, will this functionality get baked into PerlAPI's shared library? As far as I can tell, this is what Module::Build does by default.

That's a really good point and I'd never considered it. Mostly because I'd never actually gone and used multiple XS files => shared libs in the same CPAN distribution. I'd always just done INCLUDE: xs/other_file.xs to structure the code and then gotten a single .so file in the end.

I just checked the shared libs to see where the symbols from the extracted C end up:

~/perl/C-Blocks$ nm blib/arch/auto/C/Blocks/Blocks.so | grep cb_build_op
000000000000f9c0 T cb_build_op
~/perl/C-Blocks$ nm blib/arch/auto/C/Blocks/PerlAPI/PerlAPI.so | grep cb_build_op
000000000003b0f0 T cb_build_op

Alas, it's in both. :(

The linker commands show the same thing, of course, listing all the .o's.

Sorry for not realizing that before. I don't know if there's a Module::Build-friendly way of getting it to do the right thing. My fear there might not be. One can, of course, always subclass the hell out of it and manually insert special casing logic. That might be the least worst option. :( (The other option I can see is to simply do much of the compiling linking by hand, but that's obviously terrible.)

tsee avatar Jan 12 '17 13:01 tsee

https://github.com/tsee/C-Blocks/tree/hack_for_symbols

That branch has a hacky workaround.

Somewhere, it seems I broke the symbol table preprocessing, btw.

tsee avatar Jan 12 '17 13:01 tsee

Somewhere, it seems I broke the symbol table preprocessing, btw.

Turns out that I just had an old PerlAPI.xs lying around that was mysteriously not being cleaned up. I think it was an order of rebuilding and git branch switching commands that I used that confused everything. So all is well.

I think until there's a better solution, I'd actually recommend merging the hack_for_symbols branch because it avoids putting the symbols all over the place. I'll submit a pull request.

tsee avatar Jan 13 '17 12:01 tsee

When names.txt or share/perl.h.cache are not cleaned up, which happened far too frequently for my comfort, I would get terribly obscure link errors. This is in part why I had an explicit removal in the Build.PL file. Actually, it might make sense to reinstate the explicit unlink in the Build.PL file, in addition to the add_to_cleanup. Any objections?

run4flat avatar Jan 13 '17 12:01 run4flat

No objections at all. Just looked like an old debugging relic to me. :)

tsee avatar Jan 13 '17 12:01 tsee

Quick question: You mentioned you're working on splitting more code out (namely the parser). I had started on that a bit myself. Happy to leave it to you, but I was thinking about prototyping the signature scanning/parsing with the latest proposal I dumped in the other issue thread. It feels like it would be a shame if that ended up reasonable but unmergeable.

What's the status on the split? Should I just go ahead with my experimentation or wait?

(Sorry if that sounds like I'm trying to put pressure on you. Don't mean to. Just don't want to cause more harm than good.)

tsee avatar Jan 13 '17 12:01 tsee

I haven't actually worked on it yet. The reason I wanted to do it was because it would give me the opportunity to document how the thing works. If you split it out, then maybe more than one person on the planet will see how it works, and think of ways to make it a little less clever :-P

Do let me know if you take it up; I'll also notify here if I start working on it.

run4flat avatar Jan 13 '17 13:01 run4flat

I can't say that I really understand it in any level of detail. The main issue I had with how it works is that all the different pieces seem very cosy with one another, making it hard to tease them apart.

This seems primarily driven by that everything modifies state in a effectively global c_blocks_data instance and because the parser seems to want to do more than just parse. If more bits of logic were refactored to be pure(er) functions and the code-gen happened strictly after the parsing, I think a lot of this would be simpler to grok (at least for me).

I'll try and move things about a bit and push a branch (provided I have enough time to finish). I'm not convinced that what I'd be pushing would be the best or even a good way to split. But maybe it works as a starting point.

I'll let you know.

tsee avatar Jan 13 '17 13:01 tsee

https://github.com/run4flat/C-Blocks/pull/24

tsee avatar Jan 13 '17 14:01 tsee