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

Merge function calls emitted by the macro to save space. #880

Merged
merged 1 commit into from
Nov 5, 2024

Conversation

Dirbaio
Copy link
Contributor

@Dirbaio Dirbaio commented Oct 28, 2024

Save code space by merging functions.

  • If the log line has no args, generated code has 1 call instead of 3.
  • If the log line has args, generated code has n+2 calls instead of n+3.
#[inline(never)]
unsafe fn test1() {
    info!("boo");
}

#[inline(never)]
unsafe fn test2() {
    info!("boo {=u32}", 42);
}

============ BEFORE =============
000465c8 <_ZN11application5test117hcc5385f1594b37ddE>:
   465c8: b580         	push	{r7, lr}
   465ca: 466f         	mov	r7, sp
   465cc: f005 faf8    	bl	0x4bbc0 <_defmt_acquire> @ imm = #0x55f0
   465d0: 4803         	ldr	r0, [pc, #0xc]          @ 0x465e0 <_ZN11application5test117hcc5385f1594b37ddE+0x18>
   465d2: f00a f8bc    	bl	0x5074e <_ZN5defmt6export6header17hd841c3329e459a6fE> @ imm = #0xa178
   465d6: e8bd 4080    	pop.w	{r7, lr}
   465da: f005 baff    	b.w	0x4bbdc <_defmt_release> @ imm = #0x55fe
   465de: bf00         	nop
   465e0: 01 01 00 00  	.word	0x00000101

000465e4 <_ZN11application5test217h31c534e4d89d1917E>:
   465e4: b580         	push	{r7, lr}
   465e6: 466f         	mov	r7, sp
   465e8: f005 faea    	bl	0x4bbc0 <_defmt_acquire> @ imm = #0x55d4
   465ec: 4804         	ldr	r0, [pc, #0x10]         @ 0x46600 <_ZN11application5test217h31c534e4d89d1917E+0x1c>
   465ee: f00a f8ae    	bl	0x5074e <_ZN5defmt6export6header17hd841c3329e459a6fE> @ imm = #0xa15c
   465f2: 202a         	movs	r0, #0x2a
   465f4: f00a f8bc    	bl	0x50770 <_ZN5defmt6export8integers3u3217h8d5db4af4ec353d2E> @ imm = #0xa178
   465f8: e8bd 4080    	pop.w	{r7, lr}
   465fc: f005 baee    	b.w	0x4bbdc <_defmt_release> @ imm = #0x55dc
   46600: 02 01 00 00  	.word	0x00000102


============ AFTER =============
0004767c <_ZN11application5test117h23c58548f98ab538E>:
   4767c: 4801         	ldr	r0, [pc, #0x4]          @ 0x47684 <_ZN11application5test117h23c58548f98ab538E+0x8>
   4767e: f008 bcec    	b.w	0x5005a <_ZN5defmt6export26acquire_header_and_release17h4f68c6bdd10634b9E> @ imm = #0x89d8
   47682: bf00         	nop
   47684: 80 00 00 00  	.word	0x00000080

00047688 <_ZN11application5test217h6abec0c993cbeb6cE>:
   47688: b580         	push	{r7, lr}
   4768a: 466f         	mov	r7, sp
   4768c: 4804         	ldr	r0, [pc, #0x10]         @ 0x476a0 <_ZN11application5test217h6abec0c993cbeb6cE+0x18>
   4768e: f008 fcd8    	bl	0x50042 <_ZN5defmt6export18acquire_and_header17h3b663ca5e66ca628E> @ imm = #0x89b0
   47692: 202a         	movs	r0, #0x2a
   47694: f008 fcb4    	bl	0x50000 <_ZN5defmt6export8integers3u3217hc4c48e67bb670440E> @ imm = #0x8968
   47698: e8bd 4080    	pop.w	{r7, lr}
   4769c: f005 ba54    	b.w	0x4cb48 <_defmt_release> @ imm = #0x54a8
   476a0: 81 00 00 00  	.word	0x00000081

@jonathanpallant
Copy link
Contributor

This seems OK to me. Counting clock cycles, it all seems to come out in the wash performance wise because the acquire and the release still happen - they just happen elsewhere. This might even be a net performance win thanks to better cache usage.

@Urhengulas Urhengulas added the breaking change fix / feature / improvement involves a breaking change and needs to wait until next minor version label Nov 5, 2024
@Urhengulas Urhengulas added this pull request to the merge queue Nov 5, 2024
Merged via the queue into knurling-rs:main with commit 7d74e8b Nov 5, 2024
26 of 27 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking change fix / feature / improvement involves a breaking change and needs to wait until next minor version
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants