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

Adjust some definition places in kernel-common & To robust attributes in Struct #3

Closed

Conversation

kachick
Copy link
Owner

@kachick kachick commented Nov 16, 2012

I guess some definitions of methods are disused/invalid now.

@dbussink
Copy link

One comment on Struct#members. What we can probably do is merge it back to a single definition in kernel/common/struct.rb and use Rubinius::Type.convert_to_names, instead of having two versions, one with to_s and one with to_sym. We already map then to_sym always in the setup, so using convert_to_names should work fine I think.

The other stuff looks good! Good that someone is going through and cleaning up a bit :).

Before
-------
enumerator.rb: define
enumerator18.rb: none
enumerator19.rb: define

After
-----
enumerator.rb: none
enumerator18.rb: define
enumerator19.rb: define
Before
------
constant_scope.rb: define
constant_scope18.rb: define
constant_scope19.rb: define

After
-----
constant_scope.rb: none
constant_scope18.rb: define
constant_scope19.rb: define
@kachick
Copy link
Owner Author

kachick commented Nov 16, 2012

@dbussink Thanks for the pointing out!
I have used Rubinius::Type.convert_to_names in a6f82ee7f33e94cbd8e5872255be7f1fc53f173a .

But i have another question.
Does not have to use #dup in this code?

Rubinius::Type.convert_to_names(self::STRUCT_ATTRS)

or

Rubinius::Type.convert_to_names(self::STRUCT_ATTRS.dup)

@dbussink
Copy link

Dup is probably a good idea to prevent people from modifying it too easily

* Adjust definition place of Struct.#members
* Improve converting type for own version

Before
------
struct.rb: define
struct18.rb: none
struct19.rb: define

After
-----
struct.rb: define
struct18.rb: none
struct19.rb: none

Thanks for the pointing out.
----------------------------
* #3 (comment)
* bbe2d55
* 4774b58
For prevent to break inner data structures.
This is a private method.
Because accessing inner data structures.
For get consistency of coding in Struct.
@kachick
Copy link
Owner Author

kachick commented Nov 16, 2012

Thanks for the comment to dup.
And i have added some commits, for "to prevent people from modifying it too easily".

Is it more desirable that to split these commits in some pull-requests?

@dbussink
Copy link

Private _attrs seems fine, we're not a fan of freeze. Freeze is an ad hoc feature and we only have it for compatibility. We don't freeze stuff etc. in the kernel. The only places where we do is because of compatibility.

If people start modifying this constant, they are on their own. There is not a sane way to do that unintentionally.

@dbussink
Copy link

Regarding multiple pull requests, it's usually preferable to a pull request per topic. This means that discussions on certain topics don't block other pull requests. If it's all in a big pull requests, small issues would block the whole request from being merged.

@dbussink
Copy link

Oh, btw, I see this is a pull request into your own repository? Not really sure if that was intended?

@kachick
Copy link
Owner Author

kachick commented Nov 16, 2012

Sorry for the mess.
I tried to confirm of Rubinius policy, in my sandbox.

I have sent some pull-request to rubinius/rubinius.

@kachick kachick closed this Nov 16, 2012
@dbussink
Copy link

Ah, no problem :). We usually do feedback on pull requests to rubinius/rubinius too. It's fine to ask for feedback there too. BTW, we also have a very open commit policy so if you want direct commit access, just let us know!

@kachick
Copy link
Owner Author

kachick commented Nov 16, 2012

Many thanks for your teaching!
But i don't have confidence yet.

I'll try other pull-request for a while :)

@kachick kachick deleted the improve/remove_disused_methods_in_kernel-common branch December 20, 2012 01:14
kachick pushed a commit that referenced this pull request Jul 12, 2013
This fixes a crash issue where the JIT was running independent from the
GC and the GC was deallocating JIT memory at the same time. We don't
want to make the whole JIT generation GC dependent, since that causes
performance issues, so we guard all memory allocations here with a
spinlock.

The crash would be exposed with these backtraces where things were
modified concurrently:

Thread 6 (process 70553):
 #0  rubinius::jit::FreeRangeHeader::AddToFreeList () at /Users/dirkjan/Code/rubinius/vm/llvm/jit_memory_manager.hpp:151
 #1  0x000000010989f037 in rubinius::jit::MemoryRangeHeader::TrimAllocationToSize (this=0x10f7ec688, FreeList=0x10f7ec688, NewSize=5064) at vm/llvm/jit_memory_manager.cpp:211
 #2  0x000000010989bb75 in rubinius::jit::RubiniusRequestJITMemoryManager::endFunctionBody (this=<value temporarily unavailable, due to optimizations>, F=<value temporarily unavailable, due to optimizations>, FunctionStart=<value temporarily unavailable, due to optimizations>, FunctionEnd=0x13c8 <Address 0x13c8 out of bounds>) at jit_memory_manager.hpp:317
 #3  0x0000000109b4f852 in (anonymous namespace)::JITEmitter::finishFunction ()
 #4  0x0000000109946106 in (anonymous namespace)::Emitter<llvm::JITCodeEmitter>::runOnMachineFunction ()
 #5  0x0000000109bbbc30 in llvm::MachineFunctionPass::runOnFunction ()
 rubinius#6  0x0000000109f1beb2 in llvm::FPPassManager::runOnFunction ()
 rubinius#7  0x0000000109f1b9f9 in llvm::FunctionPassManagerImpl::run ()
 rubinius#8  0x0000000109f1b8a1 in llvm::FunctionPassManager::run ()
 rubinius#9  0x0000000109b461ab in llvm::JIT::runJITOnFunctionUnlocked ()
 rubinius#10 0x0000000109b46148 in llvm::JIT::runJITOnFunction ()
 rubinius#11 0x0000000109898fcc in rubinius::jit::Compiler::generate_function (this=0x10d485d38, indy=true) at vm/llvm/jit_compiler.cpp:118
 rubinius#12 0x00000001098ada93 in rubinius::BackgroundCompilerThread::perform (this=0x7fce81633240) at vm/llvm/state.cpp:345
 rubinius#13 0x00000001098ad4ef in rubinius::utilities::thread::Thread::delete_on_exit () at /Users/dirkjan/Code/rubinius/vm/util/thread.hpp:79
 rubinius#14 0x00000001098ad4ef in rubinius::utilities::thread::Thread::trampoline (arg=0x7fce81633240) at thread.hpp:211
 rubinius#15 0x00007fff8e73c7a2 in _pthread_start ()
 rubinius#16 0x00007fff8e7291e1 in thread_start ()

Thread 5 (process 70553):
 #0  0x00007fff952b5386 in __semwait_signal ()
 #1  0x00007fff8e7c6800 in nanosleep ()
 #2  0x00007fff8e7c668a in sleep ()
 #3  0x000000010969c9dd in rubinius::segv_handler (sig=11) at vm/environment.cpp:211
 #4  <signal handler called>
 #5  rubinius::jit::FreeRangeHeader::AddToFreeList () at /Users/dirkjan/Code/rubinius/vm/llvm/jit_memory_manager.hpp:151
 rubinius#6  0x000000010989ee53 in rubinius::jit::MemoryRangeHeader::FreeBlock (this=0x10f7c88f0, FreeList=<value temporarily unavailable, due to optimizations>) at jit_memory_manager.hpp:155
 rubinius#7  0x00000001098ac3e7 in rubinius::LLVMState::remove (this=<value temporarily unavailable, due to optimizations>, func=<value temporarily unavailable, due to optimizations>) at jit_memory_manager.hpp:426
 rubinius#8  0x000000010983dde9 in rubinius::CodeManager::sweep (this=0x7fce8180a2d8) at vm/gc/code_manager.cpp:107
 rubinius#9  0x0000000109750e7e in rubinius::ObjectMemory::mark () at /Users/dirkjan/Code/rubinius/vm/objectmemory.hpp:634
 rubinius#10 0x0000000109750e7e in rubinius::ObjectMemory::collect_mature_finish (this=0x7fce8180a200, state=0x10c94fec8, data=0x7fce8528b220) at vm/objectmemory.cpp:636
 rubinius#11 0x0000000109843d8a in rubinius::State::memory () at /Users/dirkjan/Code/rubinius/vm/state.hpp:171
 rubinius#12 0x0000000109843d8a in rubinius::ImmixMarker::perform (this=0x7fce8163a720, state=0x10c94fec8) at vm/gc/immix_marker.cpp:172
 rubinius#13 0x0000000109843b71 in rubinius::immix_marker_tramp (state=0x10f7ec688) at vm/gc/immix_marker.cpp:18
 rubinius#14 0x00000001098094c0 in rubinius::Thread::in_new_thread (ptr=0x7fce86a23e70) at vm/builtin/thread.cpp:250
 rubinius#15 0x00007fff8e73c7a2 in _pthread_start ()
 rubinius#16 0x00007fff8e7291e1 in thread_start ()
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants