Project

General

Profile

Actions

Feature #18397

closed

Remove documentation that Qfalse == 0 in `extension.rdoc`, instead encourage use of RTEST

Added by jemmai (Jemma Issroff) about 1 month ago. Updated about 1 month ago.

Status:
Closed
Priority:
Normal
Assignee:
-
Target version:
-
[ruby-core:106557]

Description

Currently, the extension documentation states that “Qfalse is false in C (i.e. 0).” Instead, we should encourage the use of RTEST in documentation so that we can change the value of Qfalse in the future.

Why would we want to change Qfalse away from 0?

Method calls on objects are one of the most common operations in Ruby. Each time this happens, Ruby needs to resolve the class of the object for method lookup. If the object is a Heap object, this means that Ruby must first check that the handle is a heap object pointer, so it can resolve the class by dereferencing the pointer.

Under CRuby’s current value tagging scheme, there are at least 2 comparisons necessary across 4 machine instructions to do this check. In YJIT, for example, this looks like the following:

; assume VALUE is in rax
heap_object_p:
test rax, immediate_flags
jnz not_heap_object          
cmp rax, Qnil 
jbe not_heap_object

This is because Qfalse is all 0 bits, whereas other types, like Qnil, Qtrue, flonums, etc, all have toggled bits. This means that RB_SPECIAL_CONST_P (which we negate to check if a handle is a heap pointer), has two distinct operations: RB_IMMEDIATE_P(obj) || ! RB_TEST(obj).

If, however, Qfalse was not 0, we could instead rework the value tagging scheme so that all of this could be done with one operation, similar to the existing RB_IMMEDIATE_P check. In an ideal world, the check could look something like this, where Qfalse is included in immediate_flags:

; assume VALUE is in rax
heap_object_p:
test rax, immediate_flags
jnz not_heap_object          

This could yield performance benefits in the Ruby Interpreter, YJIT and MJIT because it would speed up any method calls on an object.

Why does the current documentation make it difficult to do this?

One current barrier to changing Qfalse away from 0 is that the extension documentation guarantees that in CRuby Qfalse == 0. This means that native extensions can (and do!) rely on this fact, and use Qfalse and 0 interchangeably.

In fact, even in CRuby itself, there are several examples of when we make implicit assumptions about Qfalse. For instance, using a VALUE as the sole condition in an if statement to check for Qfalse, using MEMZERO to fill a buffer with Qfalse, using calloc to get a buffer filled with Qfalse, and many more.

How could changing the documentation help?

If we remove the documentation stating that Qfalse is 0, and instead add documentation insisting on the use of RTEST, we will position ourselves to be able to more easily change the value of Qfalse away from 0 in the future.

Proposed new documentation

See this PR or the attached patch for the specific proposed new documentation.


Files

update_qfalse_docs.patch (719 Bytes) update_qfalse_docs.patch jemmai (Jemma Issroff), 12/08/2021 06:34 PM
Actions #1

Updated by jemmai (Jemma Issroff) about 1 month ago

  • Status changed from Open to Closed

Applied in changeset git|b859397e1b25a3f7847a380e7dd7db62f94fbe66.


[DOC] Stop recommending Qfalse==0 assumption to C extensions

Encourage use of RTEST(), direct Qfalse comparison, and remove references to
Qfalse == 0 in extension documentation.

See [Bug #18397] for detail.
[ci skip]

Actions

Also available in: Atom PDF