Draft: fake PR to check build #287
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
We didn't update the includer field during compaction so it could become
a dangling pointer after compaction. It's only recently that we started
to dereference the field, and we were only comparing the pointer before
then, so the omission only recently started to cause crashes.
By instrumenting object.c:833 with `rp(includer);`, you can see the
includer field become `T_NONE` with the following script:
```ruby
mod = Module.new do
protected def foo = 1
end
klass = Class.new do
include Module.new
def run
foo
end
end
klass.include(mod)
GC.verify_compaction_references(double_heap: true, toward: :empty)
klass.new.run
```
I found a crash in a private application that this patch fixes, but
wasn't able to develop a small reproducer. Hence the above demo that
requires instrumentation.
`start` is of type uintptr_t so it does not need to be casted to VALUE.
* Update doc/format_specifications.rdoc Co-authored-by: Peter Zhu <peter@peterzhu.ca>
[0..0] internally creates an extra Array object, and so is slower and much more memory consuming ruby/logger@20616ad34a
Treats:
::pipe?
::symlink?
::socket?
::blockdev?
::chardev?
While walking over the list of subclasses for `include` and friends, we check whether the subclass is a garbage object. After the check, we allocate objects which might trigger GC and make the subclass garbage, even though before the allocation the subclass was not garbage. This is a sort of time-of-check-time-of-use issue. Fix this by saving the weak reference to a local variable, upgrading it to a strong reference while we do the allocation. It makes the code look slightly nicer even if it doesn't fix any runtime issues.
Since 4d8f762, we need to dereference the includer field on iclasses, so we need to mark it to make sure it's alive. Sometimes during compaction we crash because the field is dangling, though I have a hard time constructing such a situation. See http://ci.rvm.jp/results/trunk@ruby-iga/3947725
Object#autoload implements a custom per-thread "mutex" for blocking threads waiting on autoloading a feature. This causes problems when used with the fiber scheduler. We swap the implementation to use a Ruby mutex which is fiber aware.
to prevent the following failure on `make test-all --repeat-count=2` http://ci.rvm.jp/results/trunk-repeat20-asserts@phosphorus-docker/3957774 ``` 1) Error: TestFiberScheduler#test_autoload: NameError: uninitialized constant TestFiberSchedulerAutoload Object.const_get(:TestFiberSchedulerAutoload) ^^^^^^^^^^ ```
Having more size pools will allow us to allocate larger objects through Variable Width Allocation. I have attached some benchmark results below. Discourse: On Discourse, we don't see much change in response times. We do see a small reduction in RSS. Branch RSS: 377.8 MB Master RSS: 396.3 MB railsbench: On railsbench, we don't see a big change in RPS or p99 performance. We see a small increase in RSS. Branch RPS: 815.38 Master RPS: 811.73 Branch p99: 1.69 ms Master p99: 1.68 ms Branch RSS: 90.6 MB Master RSS: 89.4 MB liquid: We don't see a significant change in liquid performance. Branch parse & render: 29.041 I/s Master parse & render: 29.211 I/s
Depending on alignment, the last bitmap plane may not used. Then it will appear as if all of the objects on that plane is unmarked, which will cause a buffer overrun when we try to free the object. This commit changes the loop to calculate the number of planes used (bitmap_plane_count).
Size pools with no pages won't be swept so gc_sweep_finish_size_pool will never be called on it, but gc_sweep_finish_size_pool must be called to grow the size pool.
If the size pool has no or few pages/slots, then min_free_slots will be a very small number (or even 0). Then the heap won't be eligible to grow, causing GC thrashing or infinite loops.
Some size pools may not have any pages/slots, so total_slots is 0. This causes a divide-by-zero in the calculation. This commit adds a special case to catch the case when total_slots is 0 and returns the number of pages for heap_init_slots.
We don't need to allocate a new page in gc_sweep_finish_size_pool. It can be allocated when needed.
noahgibbs
force-pushed
the
noah_opt_shovel_3
branch
from
81a2c33
to
5e2ed03
Compare
For string concat, see if compile-time encoding of strings matches. If so, use simple buffer string concat at runtime. Otherwise, use encoding-checking string concat.
noahgibbs
force-pushed
the
noah_opt_shovel_3
branch
from
5e2ed03
to
cd46c77
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is an alternate method of string concat. But I'm trying to reproduce a build error.