From 98d350b6ceec8e8e3c25fb379007e64a9965537d Mon Sep 17 00:00:00 2001 From: d-netto Date: Mon, 22 Jul 2024 20:07:24 -0300 Subject: [PATCH] just use binding table --- src/ast.c | 82 ++++++++++--------------------------------- src/init.c | 1 - src/julia-syntax.scm | 45 +++++++++++------------- src/julia.h | 1 - src/julia_internal.h | 11 +++--- src/module.c | 1 - src/staticdata.c | 2 -- src/support/Makefile | 2 +- src/support/strhash.c | 40 --------------------- src/support/strhash.h | 18 ---------- 10 files changed, 47 insertions(+), 156 deletions(-) delete mode 100644 src/support/strhash.c delete mode 100644 src/support/strhash.h diff --git a/src/ast.c b/src/ast.c index 7cefa035f46cd0..bac796840c1e1d 100644 --- a/src/ast.c +++ b/src/ast.c @@ -8,8 +8,6 @@ #include #include -#include "support/strhash.h" - #ifdef _OS_WINDOWS_ #include #endif @@ -218,55 +216,17 @@ static value_t fl_nothrow_julia_global(fl_context_t *fl_ctx, value_t *args, uint decode_restriction_value(pku) : jl_atomic_load_relaxed(&b->value)) != NULL ? fl_ctx->T : fl_ctx->F; } -static htable_t *rebuild_counter_table(jl_module_t *m) JL_NOTSAFEPOINT -{ - htable_t *counter_table = m->counter_table = htable_new((htable_t *)malloc_s(sizeof(htable_t)), 0); - jl_svec_t *t = jl_atomic_load_relaxed(&m->bindings); - for (size_t i = 0; i < jl_svec_len(t); i++) { - jl_binding_t *b = (jl_binding_t*)jl_svecref(t, i); - if ((void*)b == jl_nothing) { - continue; - } - char *globalref_name = jl_symbol_name(b->globalref->name); - if (is_canonicalized_anonfn_typename(globalref_name)) { - int should_free = 1; - // copy globalref_name into the buffer until we hit a `#` character - char *delim = strchr(&globalref_name[1], '#'); - assert(delim != NULL); - size_t len = delim - globalref_name - 1; - assert(len > 0); - char *enclosing_function_name = (char*)calloc_s(len + 1); - memcpy(enclosing_function_name, &globalref_name[1], len); - // check if the enclosing function name is already in the counter table - if (strhash_get(counter_table, enclosing_function_name) == HT_NOTFOUND) { - strhash_put(counter_table, enclosing_function_name, (void*)((uintptr_t)HT_NOTFOUND + 1)); - should_free = 0; - } - char *pint = strrchr(globalref_name, '#'); - assert(pint != NULL); - int counter = atoi(pint + 1); - int max_seen_so_far = ((uint32_t)(uintptr_t)strhash_get(counter_table, enclosing_function_name) - (uintptr_t)HT_NOTFOUND - 1); - if (counter >= max_seen_so_far) { - strhash_put(counter_table, enclosing_function_name, (void*)((uintptr_t)counter + 1 + (uintptr_t)HT_NOTFOUND + 1)); - } - if (should_free) { - free(enclosing_function_name); - } - } - } - return counter_table; -} - -// used to generate a unique suffix for a given symbol (e.g. variable or type name) +// Used to generate a unique suffix for a given symbol (e.g. variable or type name) // first argument contains a stack of method definitions seen so far by `closure-convert` in flisp. // if the top of the stack is non-NIL, we use it to augment the suffix so that it becomes -// of the form $top_level_method_name##$counter, where counter is stored in a per-module -// side table indexed by top-level method name. -// this ensures that precompile statements are a bit more stable across different versions +// of the form $top_level_method_name##$counter, where `counter` is the smallest integer +// such that the resulting name is not already defined in the current module's bindings. +// If the top of the stack is NIL, we simply return the current module's counter. +// This ensures that precompile statements are a bit more stable across different versions // of a codebase. see #53719 -static value_t fl_current_module_counter(fl_context_t *fl_ctx, value_t *args, uint32_t nargs) JL_NOTSAFEPOINT +static value_t fl_module_unique_name(fl_context_t *fl_ctx, value_t *args, uint32_t nargs) { - argcount(fl_ctx, "current-julia-module-counter", nargs, 1); + argcount(fl_ctx, "julia-module-unique-name", nargs, 1); jl_ast_context_t *ctx = jl_ast_ctx(fl_ctx); jl_module_t *m = ctx->module; assert(m != NULL); @@ -275,26 +235,22 @@ static value_t fl_current_module_counter(fl_context_t *fl_ctx, value_t *args, ui value_t parsed_method_stack = args[0]; if (parsed_method_stack != fl_ctx->NIL) { value_t bottom_stack_symbol = fl_applyn(fl_ctx, 1, symbol_value(symbol(fl_ctx, "last")), parsed_method_stack); - funcname = symbol_name(fl_ctx, bottom_stack_symbol); + funcname = tosymbol(fl_ctx, bottom_stack_symbol, "julia-module-unique-name")->name; } - char buf[(funcname != NULL ? strlen(funcname) : 0) + 20]; + size_t sz = funcname != NULL ? strlen(funcname) + 32 : 32; // 32 is enough for the suffix + char *buf = (char*)alloca(sz); if (funcname != NULL && strchr(funcname, '#') == NULL) { - jl_mutex_lock_nogc(&m->lock); - htable_t *counter_table = m->counter_table; - if (counter_table == NULL) { - counter_table = rebuild_counter_table(m); - } - // try to find the function name in the module's counter table, if it's not found, add it - if (strhash_get(counter_table, funcname) == HT_NOTFOUND) { - strhash_put(counter_table, funcname, (void*)((uintptr_t)HT_NOTFOUND + 1)); + for (int i = 0; ; i++) { + snprintf(buf, sz, "%s##%d", funcname, i); + jl_sym_t *sym = jl_symbol(buf); + if (jl_get_module_binding(m, sym, 0) == NULL) { // make sure this name is not already taken + jl_get_module_binding(m, sym, 1); // create the binding + return symbol(fl_ctx, buf); + } } - uint32_t nxt = ((uint32_t)(uintptr_t)strhash_get(counter_table, funcname) - (uintptr_t)HT_NOTFOUND - 1); - snprintf(buf, sizeof(buf), "%s##%d", funcname, nxt); - strhash_put(counter_table, funcname, (void*)(nxt + 1 + (uintptr_t)HT_NOTFOUND + 1)); - jl_mutex_unlock_nogc(&m->lock); } else { - snprintf(buf, sizeof(buf), "%d", jl_module_next_counter(ctx->module)); + snprintf(buf, sz, "%d", jl_module_next_counter(m)); } return symbol(fl_ctx, buf); } @@ -329,7 +285,7 @@ static jl_value_t *scm_to_julia_(fl_context_t *fl_ctx, value_t e, jl_module_t *m static const builtinspec_t julia_flisp_ast_ext[] = { { "defined-julia-global", fl_defined_julia_global }, // TODO: can we kill this safepoint { "nothrow-julia-global", fl_nothrow_julia_global }, - { "current-julia-module-counter", fl_current_module_counter }, + { "current-julia-module-counter", fl_module_unique_name }, { "julia-scalar?", fl_julia_scalar }, { NULL, NULL } }; diff --git a/src/init.c b/src/init.c index 14090d9273fd97..86c0877b142890 100644 --- a/src/init.c +++ b/src/init.c @@ -869,7 +869,6 @@ static NOINLINE void _finish_julia_init(JL_IMAGE_SEARCH rel, jl_ptls_t ptls, jl_ jl_load(jl_core_module, "boot.jl"); post_boot_hooks(); } - jl_main_module->counter_table = NULL; if (jl_base_module == NULL) { // nthreads > 1 requires code in Base diff --git a/src/julia-syntax.scm b/src/julia-syntax.scm index 6a8fb818728d6e..6815921375184a 100644 --- a/src/julia-syntax.scm +++ b/src/julia-syntax.scm @@ -225,12 +225,11 @@ (if lb (list lb ub) (list ub)) (if lb (list lb '(core Any)) '()))))) -;; check if x is of the form (method ) or (method (outerref )) (define (is-method? x) (if (and (pair? x) (eq? (car x) 'method)) (let ((name (cadr x))) - (if (and (pair? name) (eq? (car name) 'outerref)) - (let ((name (cadr name))) + (if (and (pair? name) (eq? (car name) 'globalref)) + (let ((name (caddr name))) (if (symbol? name) #t #f)) @@ -3553,7 +3552,7 @@ f(x) = yt(x) (define (convert-lambda lam fname interp capt-sp opaq parsed-method-stack) (let ((body (add-box-inits-to-body - lam (cl-convert- (cadddr lam) fname lam (table) (table) #f interp opaq parsed-method-stack (table) (vinfo-to-table (car (lam:vinfo lam))))))) + lam (cl-convert (cadddr lam) fname lam (table) (table) #f interp opaq parsed-method-stack (table) (vinfo-to-table (car (lam:vinfo lam))))))) `(lambda ,(lam:args lam) (,(clear-capture-bits (car (lam:vinfo lam))) () @@ -3646,7 +3645,7 @@ f(x) = yt(x) (equal? rhs0 '(the_exception))) rhs0 (make-ssavalue))) - (rhs (convert-for-type-decl rhs1 (cl-convert- vt fname lam #f #f #f interp opaq parsed-method-stack (table) locals) #t lam)) + (rhs (convert-for-type-decl rhs1 (cl-convert vt fname lam #f #f #f interp opaq parsed-method-stack (table) locals) #t lam)) (ex (cond (closed `(call (core setfield!) ,(if interp `($ ,var) @@ -3933,14 +3932,14 @@ f(x) = yt(x) (define (map-cl-convert exprs fname lam namemap defined toplevel interp opaq parsed-method-stack (globals (table)) (locals (table))) (if toplevel (map (lambda (x) - (let ((tl (lift-toplevel (cl-convert- x fname lam namemap defined + (let ((tl (lift-toplevel (cl-convert x fname lam namemap defined (and toplevel (toplevel-preserving? x)) interp opaq parsed-method-stack globals locals)))) (if (null? (cdr tl)) (car tl) `(block ,@(cdr tl) ,(car tl))))) exprs) - (map (lambda (x) (cl-convert- x fname lam namemap defined #f interp opaq parsed-method-stack globals locals)) exprs))) + (map (lambda (x) (cl-convert x fname lam namemap defined #f interp opaq parsed-method-stack globals locals)) exprs))) (define (prepare-lambda! lam) ;; mark all non-arguments as assigned, since locals that are never assigned @@ -3955,7 +3954,7 @@ f(x) = yt(x) (char=? (string.char str 0) #\#) (char-numeric? (string.char str 1)))) -(define (cl-convert-- e fname lam namemap defined toplevel interp opaq parsed-method-stack (globals (table)) (locals (table))) +(define (cl-convert- e fname lam namemap defined toplevel interp opaq parsed-method-stack (globals (table)) (locals (table))) (if (and (not lam) (not (and (pair? e) (memq (car e) '(lambda method macro opaque_closure))))) (if (atom? e) e @@ -3978,7 +3977,7 @@ f(x) = yt(x) (val (if (equal? typ '(core Any)) val `(call (core typeassert) ,val - ,(cl-convert- typ fname lam namemap defined toplevel interp opaq parsed-method-stack globals locals))))) + ,(cl-convert typ fname lam namemap defined toplevel interp opaq parsed-method-stack globals locals))))) `(block ,@(if (eq? box access) '() `((= ,access ,box))) ,undefcheck @@ -4010,7 +4009,7 @@ f(x) = yt(x) e) ((=) (let ((var (cadr e)) - (rhs (cl-convert- (caddr e) fname lam namemap defined toplevel interp opaq parsed-method-stack globals locals))) + (rhs (cl-convert (caddr e) fname lam namemap defined toplevel interp opaq parsed-method-stack globals locals))) (convert-assignment var rhs fname lam interp opaq parsed-method-stack globals locals))) ((local-def) ;; make new Box for local declaration of defined variable (let ((vi (get locals (cadr e) #f))) @@ -4100,14 +4099,14 @@ f(x) = yt(x) ((null? cvs) `(block ,@sp-inits - (method ,(cadr e) ,(cl-convert- + (method ,(cadr e) ,(cl-convert ;; anonymous functions with keyword args generate global ;; functions that refer to the type of a local function (rename-sig-types sig namemap) fname lam namemap defined toplevel interp opaq parsed-method-stack globals locals) ,(let ((body (add-box-inits-to-body lam2 - (cl-convert- (cadddr lam2) 'anon lam2 (table) (table) #f interp opaq parsed-method-stack (table) + (cl-convert (cadddr lam2) 'anon lam2 (table) (table) #f interp opaq parsed-method-stack (table) (vinfo-to-table (car (lam:vinfo lam2))))))) `(lambda ,(cadr lam2) (,(clear-capture-bits (car vis)) @@ -4119,7 +4118,7 @@ f(x) = yt(x) (newlam (compact-and-renumber (linearize (car exprs)) 'none 0))) `(toplevel-butfirst (block ,@sp-inits - (method ,(cadr e) ,(cl-convert- sig fname lam namemap defined toplevel interp opaq parsed-method-stack globals locals) + (method ,(cadr e) ,(cl-convert sig fname lam namemap defined toplevel interp opaq parsed-method-stack globals locals) ,(julia-bq-macro newlam))) ,@top-stmts)))) @@ -4224,7 +4223,7 @@ f(x) = yt(x) (append (map (lambda (gs tvar) (make-assignment gs `(call (core TypeVar) ',tvar (core Any)))) closure-param-syms closure-param-names) - `((method #f ,(cl-convert- arg-defs fname lam namemap defined toplevel interp opaq parsed-method-stack globals locals) + `((method #f ,(cl-convert arg-defs fname lam namemap defined toplevel interp opaq parsed-method-stack globals locals) ,(convert-lambda lam2 (if iskw (caddr (lam:args lam2)) @@ -4284,7 +4283,7 @@ f(x) = yt(x) (block ,@body)))) ;; remaining `::` expressions are type assertions ((|::|) - (cl-convert- `(call (core typeassert) ,@(cdr e)) fname lam namemap defined toplevel interp opaq parsed-method-stack globals locals)) + (cl-convert `(call (core typeassert) ,@(cdr e)) fname lam namemap defined toplevel interp opaq parsed-method-stack globals locals)) ;; remaining `decl` expressions are only type assertions if the ;; argument is global or a non-symbol. ((decl) @@ -4292,7 +4291,7 @@ f(x) = yt(x) (local-in? (cadr e) lam locals)) '(null)) (else - (cl-convert- + (cl-convert (let ((ref (binding-to-globalref (cadr e)))) (if ref (begin @@ -4305,21 +4304,17 @@ f(x) = yt(x) fname lam namemap defined toplevel interp opaq parsed-method-stack globals locals)))) ;; `with-static-parameters` expressions can be removed now; used only by analyze-vars ((with-static-parameters) - (cl-convert- (cadr e) fname lam namemap defined toplevel interp opaq parsed-method-stack globals locals)) + (cl-convert (cadr e) fname lam namemap defined toplevel interp opaq parsed-method-stack globals locals)) (else (cons (car e) (map-cl-convert (cdr e) fname lam namemap defined toplevel interp opaq parsed-method-stack globals locals)))))))) -;; wrapper for `cl-convert--` -(define (cl-convert- e fname lam namemap defined toplevel interp opaq parsed-method-stack (globals (table)) (locals (table))) +;; wrapper for `cl-convert-` +(define (cl-convert e fname lam namemap defined toplevel interp opaq (parsed-method-stack '()) (globals (table)) (locals (table))) (if (is-method? e) (let ((name (method-expr-name e))) - (cl-convert-- e fname lam namemap defined toplevel interp opaq (cons name parsed-method-stack) globals locals)) - (cl-convert-- e fname lam namemap defined toplevel interp opaq parsed-method-stack globals locals))) - -(define (cl-convert e fname lam namemap defined toplevel interp opaq (globals (table)) (locals (table))) - (let ((parsed-method-stack '())) - (cl-convert-- e fname lam namemap defined toplevel interp opaq parsed-method-stack globals locals))) + (cl-convert- e fname lam namemap defined toplevel interp opaq (cons name parsed-method-stack) globals locals)) + (cl-convert- e fname lam namemap defined toplevel interp opaq parsed-method-stack globals locals))) (define (closure-convert e) (cl-convert e #f #f (table) (table) #f #f #f)) diff --git a/src/julia.h b/src/julia.h index d83a835d6e937f..efdd6a1b08bf75 100644 --- a/src/julia.h +++ b/src/julia.h @@ -715,7 +715,6 @@ typedef struct _jl_module_t { int8_t max_methods; jl_mutex_t lock; intptr_t hash; - htable_t *counter_table; } jl_module_t; struct _jl_globalref_t { diff --git a/src/julia_internal.h b/src/julia_internal.h index 9c9e6890814ed1..ac955e2bcb3a60 100644 --- a/src/julia_internal.h +++ b/src/julia_internal.h @@ -948,26 +948,29 @@ STATIC_INLINE jl_ptr_kind_union_t jl_walk_binding_inplace(jl_binding_t **bnd, jl } #endif +STATIC_INLINE int is10digit(char c) JL_NOTSAFEPOINT +{ + return (c >= '0' && c <= '9'); +} + STATIC_INLINE int is_anonfn_typename(char *name) { if (name[0] != '#' || name[1] == '#') return 0; char *other = strrchr(name, '#'); - return other > &name[1] && isdigit(other[1]); + return other > &name[1] && is10digit(other[1]); } // Returns true for typenames of anounymous functions that have been canonicalized (i.e. // we mangled the name of the outermost enclosing function in their name). STATIC_INLINE int is_canonicalized_anonfn_typename(char *name) JL_NOTSAFEPOINT { - if (name[0] != '#') - return 0; char *delim = strchr(&name[1], '#'); if (delim == NULL) return 0; if (delim[1] != '#') return 0; - if (!isdigit(delim[2])) + if (!is10digit(delim[2])) return 0; return 1; } diff --git a/src/module.c b/src/module.c index 64e105b6c96587..96d94049cff13d 100644 --- a/src/module.c +++ b/src/module.c @@ -66,7 +66,6 @@ JL_DLLEXPORT jl_module_t *jl_new_module_(jl_sym_t *name, jl_module_t *parent, ui jl_module_public(m, name, 1); JL_GC_POP(); } - m->counter_table = NULL; return m; } diff --git a/src/staticdata.c b/src/staticdata.c index fb194dc4641ce1..6dfe5e91a9c557 100644 --- a/src/staticdata.c +++ b/src/staticdata.c @@ -74,8 +74,6 @@ External links: #include // printf #include // PRIxPTR -#include "support/strhash.h" - #include "julia.h" #include "julia_internal.h" #include "julia_gcext.h" diff --git a/src/support/Makefile b/src/support/Makefile index a633348475f543..1ee98a4eabdeec 100644 --- a/src/support/Makefile +++ b/src/support/Makefile @@ -8,7 +8,7 @@ JCXXFLAGS += $(CXXFLAGS) JCPPFLAGS += $(CPPFLAGS) JLDFLAGS += $(LDFLAGS) -SRCS := hashing timefuncs ptrhash strhash operators utf8 ios htable bitvector \ +SRCS := hashing timefuncs ptrhash operators utf8 ios htable bitvector \ int2str libsupportinit arraylist strtod rle ifeq ($(OS),WINNT) SRCS += asprintf strptime diff --git a/src/support/strhash.c b/src/support/strhash.c deleted file mode 100644 index cbd59bed99d10b..00000000000000 --- a/src/support/strhash.c +++ /dev/null @@ -1,40 +0,0 @@ -// This file is a part of Julia. License is MIT: https://julialang.org/license - -/* - pointer hash table - optimized for storing info about particular values -*/ - -#include -#include -#include -#include -#include - -#include "dtypes.h" -#include "hashing.h" -#include "strhash.h" - -#define STR_EQ(x,y) (!strcmp((char*)(x), (char*)(y))) - -#include "htable.inc" - -#ifdef __cplusplus -extern "C" { -#endif - -size_t simple_strhash(size_t key) JL_NOTSAFEPOINT -{ - size_t hash = 0; - const char *str = (const char*)key; - while (*str) { - hash = (hash << 5) - hash + (unsigned char)*str++; - } - return hash; -} - -HTIMPL(strhash, simple_strhash, STR_EQ) - -#ifdef __cplusplus -} -#endif diff --git a/src/support/strhash.h b/src/support/strhash.h deleted file mode 100644 index cf45dcadeb4e56..00000000000000 --- a/src/support/strhash.h +++ /dev/null @@ -1,18 +0,0 @@ -// This file is a part of Julia. License is MIT: https://julialang.org/license - -#ifndef JL_STRHASH_H -#define JL_STRHASH_H - -#include "htable.h" - -#ifdef __cplusplus -extern "C" { -#endif - -HTPROT(strhash) - -#ifdef __cplusplus -} -#endif - -#endif