Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Only refer to LLVM symbol table in calls to Symbol#to_s #15486

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

HertzDevil
Copy link
Contributor

@HertzDevil HertzDevil commented Feb 18, 2025

The compiler places the string contents of all symbols defined in the source code into a special :symbol_table LLVM global variable in the main LLVM module:

@":symbol_table" = global [24 x ptr] [
  ptr @"'general'", ptr @"'no_error'", ptr @"'gc'", ptr @"'sequentially_consis...'",
  ptr @"'monotonic'", ptr @"'acquire'", ptr @"'evloop'", ptr @"'xchg'", ptr @"'release'",
  ptr @"'acquire_release'", ptr @"'io_write'", ptr @"'sched'", ptr @"'io_read'",
  ptr @"'skip'", ptr @"'none'", ptr @"'active'", ptr @"'done'", ptr @"'unchecked'",
  ptr @"'sleep'", ptr @"'relaxed'", ptr @"'add'", ptr @"'sub'", ptr @"'file'", ptr @"'target'"
]

Every LLVM module, without exception, also declares it:

@":symbol_table" = external global [24 x ptr]

Since the total number of symbols is part of the variable type, adding or removing any symbol invalidates all object files in the Crystal cache, even though only Symbol#to_s, or rather @[Primitive(:symbol_to_s)], has access to this variable.

This PR removes this declaration except where the primitive accesses it. Changes to the symbol table will only affect LLVM modules that call Symbol#to_s (the primitive body itself is inlined). Building an empty file, replacing it with x = :abcde, then rebuilding the same file will now give:

Codegen (bc+obj):
 - 315/317 .o files were reused

These modules were not reused:
 - _main (_main.o0.bc)
 - Crystal (C-rystal.o0.bc)

(Crystal is also invalidated because the return type of Crystal.main is now Symbol instead of Nil; appending nil to the file makes this module reusable.)

Note that it is still possible to invalidate large portions of the cache from the addition or removal of symbols, because their indices are allocated sequentially, and inlined on every use. Something like #15485 but for Symbol values would solve that.

@HertzDevil HertzDevil changed the title Only refer to symbol table in definition of Symbol#to_s Only refer to LLVM symbol table in calls to Symbol#to_s Feb 18, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants