| Age | Commit message (Collapse) | Author |
|---|
|
* Prefixed ccan headers
* Remove unprefixed names in ccan/build_assert
* Remove unprefixed names in ccan/check_type
* Remove unprefixed names in ccan/container_of
* Remove unprefixed names in ccan/list
Co-authored-by: Samuel Williams <samuel.williams@oriontransfer.co.nz>
Notes:
Merged-By: ioquatix <samuel@codeotaku.com>
|
|
Use ISEQ_BODY macro to get the rb_iseq_constant_body of the ISeq. Using
this macro will make it easier for us to change the allocation strategy
of rb_iseq_constant_body when using Variable Width Allocation.
Notes:
Merged: https://github.com/ruby/ruby/pull/5698
|
|
* Rename --jit to --mjit
[Feature #18349]
* Fix a few more --jit references
* Fix MJIT Actions
* More s/jit/mjit/ and re-introduce --disable-jit
* Update NEWS.md
* Fix test_bug_reporter_add
Notes:
Merged-By: k0kubun <takashikkbn@gmail.com>
|
|
With MSVC, MJIT uses the /Fd option on an installed PDB file when
compiling. Combined with the /Zi option, this causes the PDB file to be
modified every time MJIT compiles. Concurrent modifications to the same
PDB file is known to cause problems. MSVC even has an option, /FS to
deal with it. When running MJIT tests in parallel, sometimes this leads
to corrupting the PDB file, breaking subsequent compilations. On CI,
we get messages like these:
rb_mjit_header-3.1.0.pdb is not the pdb file that was used when this precompiled header was created, recreate the precompiled header.
To avoid this race, use the /Z7 option when building precompiled header,
which asks the compiler to put debug info into the .obj file,
eliminating the need for pointing the compiler to the PDB file for the
precompiled header.
The /Fd option is changed to use a unique path based on the name of the
dll output. Because of the /debug linker flag, we generate a PDB file
at runtime even though we use /Z7.
There are a couple things missing from this change:
- Because MJIT uses the interpreter's CFLAGS build option and that
contains /Zi, putting /Z7 at the end leads to a build warning
- With /Z7 no PDB file is built anymore, so the code for installing
the PDB file can be removed
There might also be other problems with this change I haven't noticed
while developing this change using Github Actions. I don't have a
Windows dev environment with Visual Studio so I can't finish this
change easily. Please feel free to complete this change if it makes
sense.
Note:
- On master, you can see the PDB file changing with llvm-pdbutil or a
simple checksum. There is an age field in the file that is bumped
- I'm not sure if users can specify compile flags on MSVC. If they
couldn't, maybe it's easier to change MJIT's compile options to
use /Z7 when building the precompile header.
- MJIT could pass different options at runtime to generate fewer
files. Right now it inherits the /DEBUG linker flag which causes
a PDB file to be generated at runtime even though /Z7 is used.
Relevant MSVC docs:
- [/Zi,/Z7](https://docs.microsoft.com/en-us/cpp/build/reference/z7-zi-zi-debug-information-format?view=msvc-160)
- [/DEBUG](https://docs.microsoft.com/en-us/cpp/build/reference/debug-generate-debug-info?view=msvc-160)
- [/FS](https://docs.microsoft.com/en-us/cpp/build/reference/fs-force-synchronous-pdb-writes?view=msvc-160)
Notes:
Merged: https://github.com/ruby/ruby/pull/5058
|
|
* --braces-after-func-def-line
* --dont-cuddle-else
* --procnames-start-lines
* --space-after-for
* --space-after-if
* --space-after-while
|
|
by a race condition by multiple Ractors.
Atmically incrementing body->total_calls may have its own cost, so for
now we intentionally leave the unreliable total_calls. So we allow an
ISeq to be never pushed when you use multiple Ractors. However, if you
enqueue a single ccan node twice, get_from_list loops infinitely. Thus
this patch takes care of such a situation.
|
|
* Improve perfomance for Integer#size method [Feature #17135]
* re-run ci
* Let MJIT frame skip work for Integer#size
Co-authored-by: Takashi Kokubun <takashikkbn@gmail.com>
Notes:
Merged-By: k0kubun <takashikkbn@gmail.com>
|
|
This is a follow-up of 86c262541ad07528842d76dab4b9b34bd888d5f4.
CRITICAL_SECTION_START/FINISH are not needed when it's called from an
MJIT worker.
Also, ZALLOC needs to be calloc because ZALLOC may trigger GC, which an
MJIT worker must not do.
|
|
This fixes SEGVs like https://github.com/ruby/ruby/runs/2715166621?check_suite_focus=true.
When mjit_recompile is called when mjit_compile is compiling the exact
same iseq (and after it called mjit_capture_cc_entries), iseq->body->jit_unit
is re-created and its cc_entries becomes NULL. Then, when it tries to
lookup cc_entries through iseq->body->jit_unit, it fails.
|
|
It's been a way too much amount of ifdefs.
|
|
https://github.com/ruby/ruby/runs/2707566811#step:10:147
```
D:\a\ruby\ruby\src\mjit_worker.c(1212): warning C4090: 'function': different 'const' qualifiers
```
|
|
This is useful for large applications like Rails.
https://k0kubun.medium.com/ruby-3-jit-can-make-rails-faster-756310f235a
|
|
[Bug #17584]
Notes:
Merged-By: k0kubun <takashikkbn@gmail.com>
|
|
Because we want to flush pending stale units before unloading units, the
pending_stale_p check is implemented in this waiting loop.
However, once all methods are called more than --jit-min-calls,
mjit_worker_wakeup will not be signaled again. As a result, when
mjit_recompile is called after that and pending_stale_p becomes true,
MJIT stops processing methods in the unit queue even if the queue is
very long and MJIT does nothing, waiting for the signal.
There should be a better way to handle this, but as a fix to be
backported to Ruby 3.0, let me make an obvious simple commit here.
|
|
[Bug #17836]
Notes:
Merged-By: k0kubun <takashikkbn@gmail.com>
|
|
When a worker is stopped, nobody will JIT a method for you.
|
|
`mjit_valid_class_serial_p` has no longer been used since b9007b6c548.
|
|
This is to avoid SEGV on a CC reference in a normal compilation
https://github.com/ruby/ruby/runs/1586578023
|
|
to avoid SEGV like
http://ci.rvm.jp/results/trunk-mjit@phosphorus-docker/3289588
by a race condition between mjit_recompile and compation around active_units
|
|
556a7285080c1344c75bb93a333c9bfc5d631c61 was not good maybe because it
wasn't using list_for_each_safe. If list_for_each_safe is safe for
list_del for any nodes (is that true?), this should be fine.
|
|
To fix
http://ci.rvm.jp/results/trunk-mjit-wait@phosphorus-docker/3286265
|
|
This reverts commit b7dc04e51823f9fe8b5355c30a304ecdb11fe5ed.
This should be fine, rather necessary, too.
|
|
This reverts commit 73b07c437e24711c23dd2dd01d3ffc5f1012e046.
This was, of course, innocent.
|
|
list_for_each seems to cause all the SEGVs we've seen.
|
|
Revert "Lock the entire active_units loop"
This reverts commit 5c2ff88be2e515613dfe54823e8429656f688e9f.
Revert "Lock active_units references on compaction"
This reverts commit 556a7285080c1344c75bb93a333c9bfc5d631c61.
Revert "Wait for GC before unload_units"
This reverts commit a8f16df615daa55901bb351efe038e86b61fbb92.
Well, the previous revert actually didn't fix it, but this series of
reverts seems to rollback the situation a little.
|
|
This reverts commit 3319ce37651aa7e50c31b5fba14871938318b37a.
I still haven't figured out why, but this seems to have increased the
failure rate.
|
|
|
|
The previous fix seems not working. Let me test if this works.
|
|
to convert list_node to rb_mjit_unit easily in gdb.
|
|
This might race with mjit_recompile.
|
|
--jit-wait CI can be stuck when the workaround is throttled
http://ci.rvm.jp/results/trunk-mjit-wait@phosphorus-docker/3274091
|
|
The compilation for JIT compaction is very heavy. Triggering a second
compaction to include one more new method is probably not worth it. So
this triggers JIT compaction for ten more new methods after each
compaction.
|
|
Because d80226e7bd often reduces the number of unloaded units, it
increases the number of unload_units calls, which are heavy.
To mitigate that, this throttles unload_units per `max_cache_size / 10`.
Also hoping to fix
https://ci.appveyor.com/project/ruby/ruby/builds/36552382/job/kjmjgw9cjyf2ksd7
|
|
instead of just unloading worst 10% methods.
|
|
|
|
to avoid "Too many JIT code, but skipped unloading units for JIT compaction".
Now we can forget the `in_compact` locking.
Moving some functions from mjit.c to mjit_worker.c because mjit_worker.c
should have functions executed in the JIT worker.
|
|
|
|
* Re-generate C files for JIT compaction every time
* Refactor in_jit return logic
* Just write code in a single file
* Add a TODO comment [ci skip]
Notes:
Merged-By: k0kubun <takashikkbn@gmail.com>
|
|
This was a leftover of 27d5af59a359909e0d434459c30cfc0940f60a5b.
|
|
convert_unit_to_func's c_func / so_func construction is unnecessarily
complicated while it's not really safer than what compact_all_jit_code
does. So I changed convert_unit_to_func to be consistent with
compact_all_jit_code.
|
|
This has been a TODO since 79df14c04b. While adcf0316d1 covered the
root_fiber of the initial thread, it didn't cover root_fibers of other
threads. Now it's hooked properly in rb_threadptr_root_fiber_setup.
With regards to "XXX: Is this mjit_cont `mjit_cont_free`d?", when
rb_threadptr_root_fiber_release is called, although I'm not sure when
th->root_fiber is truthy, fiber_free seems to call cont_free and
mjit_cont_free. So mjit_conts of root_fibers seem to be freed properly.
|
|
It's calculated inside compile_c_to_so again.
|
|
Apparently #ifdef is always true
|
|
_MSC_VER used to be the macro to switch JIT compaction. However, since
d4381d2ceb, the correct macro to switch it was changed from _MSC_VER
to _WIN32. As I didn't properly replace all relevant _MSC_VER usages
to _WIN32, these macros have been used inconsistently.
nobu replaced _WIN32 with USE_HEADER_TRANSFORMATION in 5eb446d12f3.
Therefore we had USE_HEADER_TRANSFORMATION and _MSC_VER. This commit
makes sure such inconsistent _MSC_VER usages will be unified to the new
header, also renaming it to USE_JIT_COMPACTION to be more precise about
the requirements. The header transformation itself is not quite relevant
to places changed in this commit.
|
|
Isn't setting `in_compact = true` enough to avoid a race condition
between JIT compaction and unload_units? Now I think it is.
This change will make it easier to spend more time on compile_compact_jit_code.
For now it seems to take only 0.0723ms though.
|
|
Thanks to Ractor (https://github.com/ruby/ruby/pull/2888 and https://github.com/ruby/ruby/pull/3662),
inline caches support parallel access now.
Notes:
Merged-By: k0kubun <takashikkbn@gmail.com>
|
|
This reverts commit 6cb6d5abc36ede9d5158c2cd90734134838e6bfb.
This reverts commit 1484b786aee8d411a9e2278ac6d6e44aedbf6662.
I think we don't need these assertions anymore. I believe the problem
is solved by abf678a4397c6c00a1bb686043e377d372e695a4
|
|
We are seeing an error where code that is generated with MJIT contains
references to objects that have been moved. I believe this is due to a
race condition in the compaction function.
`gc_compact` has two steps:
1. Run a full GC to pin objects
2. Compact / update references
Step one is executed with `garbage_collect`. `garbage_collect` calls
`gc_enter` / `gc_exit`, these functions acquire a JIT lock and release a
JIT lock. So a lock is held for the duration of step 1.
Step two is executed by `gc_compact_after_gc`. It also holds a JIT
lock.
I believe the problem is that the JIT is free to execute between step 1
and step 2. It copies call cache values, but doesn't pin them when it
copies them. So the compactor thinks it's OK to move the call cache
even though it is not safe.
We need to hold a lock for the duration of `garbage_collect` *and*
`gc_compact_after_gc`. This patch introduces a lock level which
increments and decrements. The compaction function can increment and
decrement the lock level and prevent MJIT from executing during both
steps.
Notes:
Merged: https://github.com/ruby/ruby/pull/3683
|
|
```
compiling ../mjit.c
In file included from ../mjit.c:28:
../mjit_worker.c:1270:33: warning: incompatible pointer to integer conversion passing 'const struct rb_callcache *' to parameter of type 'VALUE' (aka 'unsigned long') [-Wint-conversion]
assert(BUILTIN_TYPE(cc) != T_MOVED);
^~
/Library/Developer/CommandLineTools/SDKs/MacOSX10.14.sdk/usr/include/assert.h:93:25: note: expanded from macro 'assert'
(__builtin_expect(!(e), 0) ? __assert_rtn(__func__, __FILE__, __LINE__, #e) : (void)0)
^
../include/ruby/internal/value_type.h:153:23: note: passing argument to parameter 'obj' here
RB_BUILTIN_TYPE(VALUE obj)
^
In file included from ../mjit.c:28:
../mjit_worker.c:1271:33: warning: incompatible pointer to integer conversion passing 'const struct rb_callable_method_entry_struct *' to parameter of type 'VALUE' (aka 'unsigned long') [-Wint-conversion]
assert(BUILTIN_TYPE(vm_cc_cme(cc)) != T_MOVED);
^~~~~~~~~~~~~
/Library/Developer/CommandLineTools/SDKs/MacOSX10.14.sdk/usr/include/assert.h:93:25: note: expanded from macro 'assert'
(__builtin_expect(!(e), 0) ? __assert_rtn(__func__, __FILE__, __LINE__, #e) : (void)0)
^
../include/ruby/internal/value_type.h:153:23: note: passing argument to parameter 'obj' here
RB_BUILTIN_TYPE(VALUE obj)
^
In file included from ../mjit.c:28:
../mjit_worker.c:1272:50: warning: incompatible pointer to integer conversion passing 'const struct rb_callcache *' to parameter of type 'VALUE' (aka 'unsigned long') [-Wint-conversion]
assert(!rb_objspace_garbage_object_p(cc));
^~
/Library/Developer/CommandLineTools/SDKs/MacOSX10.14.sdk/usr/include/assert.h:93:25: note: expanded from macro 'assert'
(__builtin_expect(!(e), 0) ? __assert_rtn(__func__, __FILE__, __LINE__, #e) : (void)0)
^
../gc.h:128:40: note: passing argument to parameter 'obj' here
int rb_objspace_garbage_object_p(VALUE obj);
^
In file included from ../mjit.c:28:
../mjit_worker.c:1273:50: warning: incompatible pointer to integer conversion passing 'const struct rb_callable_method_entry_struct *' to parameter of type 'VALUE' (aka 'unsigned long') [-Wint-conversion]
assert(!rb_objspace_garbage_object_p(vm_cc_cme(cc)));
^~~~~~~~~~~~~
/Library/Developer/CommandLineTools/SDKs/MacOSX10.14.sdk/usr/include/assert.h:93:25: note: expanded from macro 'assert'
(__builtin_expect(!(e), 0) ? __assert_rtn(__func__, __FILE__, __LINE__, #e) : (void)0)
^
../gc.h:128:40: note: passing argument to parameter 'obj' here
int rb_objspace_garbage_object_p(VALUE obj);
^
4 warnings generated.
```
|
|
This is a temporary commit to try to find a GC issue. It seems like
mjit is pointing at a moved address in the call cache. I want to assert
that they aren't TMOVED or garbage objects at the time they get copied
|
