Skip to content

Commit 20d6c03

Browse files
cthorn42cthorn42
authored
Merge pull request #785 from joshcooper/redo_windows_realpath_path
(PA-6051) Revert patch for ruby 3.2.3 on Windows
2 parents 0103919 + 85059be commit 20d6c03

File tree

2 files changed

+146
-7
lines changed

2 files changed

+146
-7
lines changed

‎configs/components/ruby-3.2.3.rb

+1-7
Original file line numberDiff line numberDiff line change
@@ -38,14 +38,8 @@
3838

3939
base = 'resources/patches/ruby_32'
4040

41-
4241
if platform.is_cross_compiled?
43-
unless platform.is_macos?
44-
# pkg.apply_patch "#{base}/Replace-reference-to-RUBY-var-with-opt-pl-build-tool.patch"
45-
end
46-
4742
pkg.apply_patch "#{base}/rbinstall_gem_path.patch"
48-
# pkg.apply_patch "#{base}/revert_host_value_changes.patch"
4943
end
5044

5145
if platform.is_aix?
@@ -56,6 +50,7 @@
5650
pkg.apply_patch "#{base}/windows_mingw32_mkmf.patch"
5751
pkg.apply_patch "#{base}/windows_nocodepage_utf8_fallback_r2.5.patch"
5852
pkg.apply_patch "#{base}/ruby-faster-load_32.patch"
53+
pkg.apply_patch "#{base}/revert_speed_up_rebuilding_loaded_feature_index.patch"
5954
pkg.apply_patch "#{base}/revert-ruby-double-load-symlink.patch"
6055
pkg.apply_patch "#{base}/revert_ruby_utf8_default_encoding.patch"
6156
end
@@ -67,7 +62,6 @@
6762
if platform.is_macos?
6863
pkg.environment 'optflags', settings[:cflags]
6964
pkg.environment 'PATH', '$(PATH):/opt/homebrew/bin:/usr/local/bin'
70-
7165
elsif platform.is_windows?
7266
pkg.environment 'optflags', settings[:cflags] + ' -O3'
7367
pkg.environment 'MAKE', 'make'

‎resources/patches/ruby_32/revert_speed_up_rebuilding_loaded_feature_index.patch

+145
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,145 @@
1+
commit 6c6b5199a4bb18d09b1da255b5c8bd549b03dc42
2+
Author: Josh Cooper <[email protected]>
3+
Date: Wed Jan 24 22:31:57 2024 +0000
4+
5+
Revert "merge revision(s) 1f115f141dd17f75049a5e17107906c5bcc372e1:"
6+
7+
This reverts commit 5baf94f9131fb45d50c8c408e007a138ced46606.
8+
9+
Commit 79a4484a072e9769b603e7b4fbdb15b1d7eccb15 resulted in many more calls to
10+
`rb_realpath` when trying to load a file.
11+
12+
Commit 8346d1630b8193eef1ec9dd537b16de74afdc2e8 added a cache to fix that.
13+
It was reverted in commit 1c7624469880bcb964be09a49e4907873f45b026.
14+
15+
Commit 788b03d5ba82fd8b35ce1fe2618ce6bacc648333 added a second fix.
16+
It was reverted in e777064e4b064fd77aca65c123f1919433f6732d.
17+
18+
Commit 5baf94f9131fb45d50c8c408e007a138ced46606 added a third fix and released in 3.2.3.
19+
The change does reduce the number of file I/O syscalls on RHEL 8:
20+
21+
puppet-agent 8.4.0 puppet-agent 8.4.0
22+
ruby 3.2.2 w/o patches ruby 3.2.3 w/o patches
23+
calls errors calls errors
24+
25+
puppet --version 57996 5955 34511 6069
26+
27+
puppet apply -e "" 455006 18845 55159 19089
28+
29+
In both scenarios, the number of file syscalls dropped from:
30+
31+
57996 -> 34511 (59.5%)
32+
455006 -> 55159 (12.1%)
33+
34+
And the time to apply an empty catalog dropped from 1.908 seconds to 1.435 seconds.
35+
36+
However, platforms like Windows don't HAVE_REALPATH. Instead ruby emulates it
37+
by walking each ancestor directory. So revert the third attempt to speed up the
38+
feature index and realpath cache so that we can revert 79a4484a072e9769b603e7b4fbdb15b1d7eccb15
39+
40+
diff --git a/load.c b/load.c
41+
index b2e5e962c8..f0219fcf50 100644
42+
--- a/load.c
43+
+++ b/load.c
44+
@@ -163,12 +163,6 @@ get_loaded_features_realpaths(rb_vm_t *vm)
45+
return vm->loaded_features_realpaths;
46+
}
47+
48+
-static VALUE
49+
-get_loaded_features_realpath_map(rb_vm_t *vm)
50+
-{
51+
- return vm->loaded_features_realpath_map;
52+
-}
53+
-
54+
static VALUE
55+
get_LOADED_FEATURES(ID _x, VALUE *_y)
56+
{
57+
@@ -367,10 +361,7 @@ get_loaded_features_index(rb_vm_t *vm)
58+
st_foreach(vm->loaded_features_index, loaded_features_index_clear_i, 0);
59+
60+
VALUE realpaths = vm->loaded_features_realpaths;
61+
- VALUE realpath_map = vm->loaded_features_realpath_map;
62+
- VALUE previous_realpath_map = rb_hash_dup(realpath_map);
63+
rb_hash_clear(realpaths);
64+
- rb_hash_clear(realpath_map);
65+
features = vm->loaded_features;
66+
for (i = 0; i < RARRAY_LEN(features); i++) {
67+
VALUE entry, as_str;
68+
@@ -387,14 +378,9 @@ get_loaded_features_index(rb_vm_t *vm)
69+
long j = RARRAY_LEN(features);
70+
for (i = 0; i < j; i++) {
71+
VALUE as_str = rb_ary_entry(features, i);
72+
- VALUE realpath = rb_hash_aref(previous_realpath_map, as_str);
73+
- if (NIL_P(realpath)) {
74+
- realpath = rb_check_realpath(Qnil, as_str, NULL);
75+
- if (NIL_P(realpath)) realpath = as_str;
76+
- realpath = rb_fstring(realpath);
77+
- }
78+
- rb_hash_aset(realpaths, realpath, Qtrue);
79+
- rb_hash_aset(realpath_map, as_str, realpath);
80+
+ VALUE realpath = rb_check_realpath(Qnil, as_str, NULL);
81+
+ if (NIL_P(realpath)) realpath = as_str;
82+
+ rb_hash_aset(realpaths, rb_fstring(realpath), Qtrue);
83+
}
84+
}
85+
return vm->loaded_features_index;
86+
@@ -1176,7 +1162,6 @@ require_internal(rb_execution_context_t *ec, VALUE fname, int exception, bool wa
87+
volatile VALUE saved_path;
88+
volatile VALUE realpath = 0;
89+
VALUE realpaths = get_loaded_features_realpaths(th->vm);
90+
- VALUE realpath_map = get_loaded_features_realpath_map(th->vm);
91+
volatile bool reset_ext_config = false;
92+
struct rb_ext_config prev_ext_config;
93+
94+
@@ -1267,9 +1252,7 @@ require_internal(rb_execution_context_t *ec, VALUE fname, int exception, bool wa
95+
rb_provide_feature(th2->vm, path);
96+
VALUE real = realpath;
97+
if (real) {
98+
- real = rb_fstring(real);
99+
- rb_hash_aset(realpaths, real, Qtrue);
100+
- rb_hash_aset(realpath_map, path, real);
101+
+ rb_hash_aset(realpaths, rb_fstring(real), Qtrue);
102+
}
103+
}
104+
ec->errinfo = saved.errinfo;
105+
@@ -1498,8 +1481,6 @@ Init_load(void)
106+
vm->loaded_features_index = st_init_numtable();
107+
vm->loaded_features_realpaths = rb_hash_new();
108+
rb_obj_hide(vm->loaded_features_realpaths);
109+
- vm->loaded_features_realpath_map = rb_hash_new();
110+
- rb_obj_hide(vm->loaded_features_realpath_map);
111+
112+
rb_define_global_function("load", rb_f_load, -1);
113+
rb_define_global_function("require", rb_f_require, 1);
114+
diff --git a/vm.c b/vm.c
115+
index de43d022c0..d009a5f64a 100644
116+
--- a/vm.c
117+
+++ b/vm.c
118+
@@ -2703,7 +2703,6 @@ rb_vm_update_references(void *ptr)
119+
vm->loaded_features = rb_gc_location(vm->loaded_features);
120+
vm->loaded_features_snapshot = rb_gc_location(vm->loaded_features_snapshot);
121+
vm->loaded_features_realpaths = rb_gc_location(vm->loaded_features_realpaths);
122+
- vm->loaded_features_realpath_map = rb_gc_location(vm->loaded_features_realpath_map);
123+
vm->top_self = rb_gc_location(vm->top_self);
124+
vm->orig_progname = rb_gc_location(vm->orig_progname);
125+
126+
@@ -2795,7 +2794,6 @@ rb_vm_mark(void *ptr)
127+
rb_gc_mark_movable(vm->loaded_features);
128+
rb_gc_mark_movable(vm->loaded_features_snapshot);
129+
rb_gc_mark_movable(vm->loaded_features_realpaths);
130+
- rb_gc_mark_movable(vm->loaded_features_realpath_map);
131+
rb_gc_mark_movable(vm->top_self);
132+
rb_gc_mark_movable(vm->orig_progname);
133+
RUBY_MARK_MOVABLE_UNLESS_NULL(vm->coverages);
134+
diff --git a/vm_core.h b/vm_core.h
135+
index b6adeadd87..d86fdbaecd 100644
136+
--- a/vm_core.h
137+
+++ b/vm_core.h
138+
@@ -680,7 +680,6 @@ typedef struct rb_vm_struct {
139+
VALUE loaded_features;
140+
VALUE loaded_features_snapshot;
141+
VALUE loaded_features_realpaths;
142+
- VALUE loaded_features_realpath_map;
143+
struct st_table *loaded_features_index;
144+
struct st_table *loading_table;
145+
#if EXTSTATIC

0 commit comments

Comments
 (0)