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

use dump but fast function for whole string hashing #1

Closed

Conversation

funny-falcon
Copy link

it fits into i386 registers, and consumes more than 2 bytes per cycle
(on Haswell)

  • add check for hashsum equality in chain traversion.

it fits into i386 registers, and consumes more than 2 bytes per cycle
(on Haswell)

+ add check for hashsum equality in chain traversion.
Buristan pushed a commit that referenced this pull request Dec 11, 2023
Thanks to Sergey Kaplun and Peter Cawley.

(cherry-picked from commit 1761fd2)

Assume we have the parent trace with the following IRs:

| 0001 }  tab TNEW   #0    #0
| 0002 }  p32 NEWREF 0001  "key"
| 0003 }  fal HSTORE 0002  false
| ....        SNAP   #1   [ ---- ---- 0001 ---- ]
| 0004 >  num SLOAD  #1    T
| ....        SNAP   #2   [ ---- ---- 0001 ]
| 0005 >  num EQ     0004  0004
| 0006 }  tru HSTORE 0002  true
| ....        SNAP   #3   [ ---- ---- 0001 true ]

The side trace for the third snapshot emits the following IRs:

| 0001    tab TNEW   #0    #0
| 0002    p32 NEWREF 0001  "key"
| 0003    fal HSTORE 0002  false
| 0004    p32 NEWREF 0001  "key"
| 0005    tru HSTORE 0004  true

As we can see, `NEWREF` is emitted twice. This is a violation of its
semantics, so the second store isn't noticeable.

This patch prevents the second emitting of IR NEWREF by checking the last
one emitted NEWREF IR. There is no need to check NEWREFs beyond since
it guarantees the snapshot is taken after it, because it may cause table
rehashing, so all prior results are invalidated.

Sergey Kaplun:
* added the description and the test for the problem

Resolves tarantool/tarantool#7937
Part of tarantool/tarantool#9145
Buristan pushed a commit that referenced this pull request Dec 12, 2023
Thanks to Sergey Kaplun and Peter Cawley.

(cherry-picked from commit 1761fd2)

Assume we have the parent trace with the following IRs:

| 0001 }  tab TNEW   #0    #0
| 0002 }  p32 NEWREF 0001  "key"
| 0003 }  fal HSTORE 0002  false
| ....        SNAP   #1   [ ---- ---- 0001 ---- ]
| 0004 >  num SLOAD  #1    T
| ....        SNAP   #2   [ ---- ---- 0001 ]
| 0005 >  num EQ     0004  0004
| 0006 }  tru HSTORE 0002  true
| ....        SNAP   #3   [ ---- ---- 0001 true ]

The side trace for the third snapshot emits the following IRs:

| 0001    tab TNEW   #0    #0
| 0002    p32 NEWREF 0001  "key"
| 0003    fal HSTORE 0002  false
| 0004    p32 NEWREF 0001  "key"
| 0005    tru HSTORE 0004  true

As we can see, `NEWREF` is emitted twice. This is a violation of its
semantics, so the second store isn't noticeable.

This patch prevents the second emitting of IR NEWREF by checking the last
one emitted NEWREF IR. There is no need to check NEWREFs beyond since
it guarantees the snapshot is taken after it, because it may cause table
rehashing, so all prior results are invalidated.

Sergey Kaplun:
* added the description and the test for the problem

Resolves tarantool/tarantool#7937
Part of tarantool/tarantool#9145
igormunkin pushed a commit that referenced this pull request Jan 29, 2024
Thanks to Sergey Kaplun and Peter Cawley.

(cherry-picked from commit 1761fd2)

Assume we have the parent trace with the following IRs:

| 0001 }  tab TNEW   #0    #0
| 0002 }  p32 NEWREF 0001  "key"
| 0003 }  fal HSTORE 0002  false
| ....        SNAP   #1   [ ---- ---- 0001 ---- ]
| 0004 >  num SLOAD  #1    T
| ....        SNAP   #2   [ ---- ---- 0001 ]
| 0005 >  num EQ     0004  0004
| 0006 }  tru HSTORE 0002  true
| ....        SNAP   #3   [ ---- ---- 0001 true ]

The side trace for the third snapshot emits the following IRs:

| 0001    tab TNEW   #0    #0
| 0002    p32 NEWREF 0001  "key"
| 0003    fal HSTORE 0002  false
| 0004    p32 NEWREF 0001  "key"
| 0005    tru HSTORE 0004  true

As we can see, `NEWREF` is emitted twice. This is a violation of its
semantics, so the second store isn't noticeable.

This patch prevents the second emitting of IR NEWREF by checking the last
one emitted NEWREF IR. There is no need to check NEWREFs beyond since
it guarantees the snapshot is taken after it, because it may cause table
rehashing, so all prior results are invalidated.

Sergey Kaplun:
* added the description and the test for the problem

Resolves tarantool/tarantool#7937
Part of tarantool/tarantool#9145
igormunkin pushed a commit that referenced this pull request Jan 29, 2024
Thanks to Sergey Kaplun and Peter Cawley.

(cherry-picked from commit 1761fd2)

Assume we have the parent trace with the following IRs:

| 0001 }  tab TNEW   #0    #0
| 0002 }  p32 NEWREF 0001  "key"
| 0003 }  fal HSTORE 0002  false
| ....        SNAP   #1   [ ---- ---- 0001 ---- ]
| 0004 >  num SLOAD  #1    T
| ....        SNAP   #2   [ ---- ---- 0001 ]
| 0005 >  num EQ     0004  0004
| 0006 }  tru HSTORE 0002  true
| ....        SNAP   #3   [ ---- ---- 0001 true ]

The side trace for the third snapshot emits the following IRs:

| 0001    tab TNEW   #0    #0
| 0002    p32 NEWREF 0001  "key"
| 0003    fal HSTORE 0002  false
| 0004    p32 NEWREF 0001  "key"
| 0005    tru HSTORE 0004  true

As we can see, `NEWREF` is emitted twice. This is a violation of its
semantics, so the second store isn't noticeable.

This patch prevents the second emitting of IR NEWREF by checking the last
one emitted NEWREF IR. There is no need to check NEWREFs beyond since
it guarantees the snapshot is taken after it, because it may cause table
rehashing, so all prior results are invalidated.

Sergey Kaplun:
* added the description and the test for the problem

Resolves tarantool/tarantool#7937
Part of tarantool/tarantool#9145

Reviewed-by: Maxim Kokryashkin <[email protected]>
Reviewed-by: Sergey Bronnikov <[email protected]>
Signed-off-by: Igor Munkin <[email protected]>
(cherry picked from commit 005e8ce)
igormunkin pushed a commit that referenced this pull request Jan 29, 2024
Thanks to Sergey Kaplun and Peter Cawley.

(cherry-picked from commit 1761fd2)

Assume we have the parent trace with the following IRs:

| 0001 }  tab TNEW   #0    #0
| 0002 }  p32 NEWREF 0001  "key"
| 0003 }  fal HSTORE 0002  false
| ....        SNAP   #1   [ ---- ---- 0001 ---- ]
| 0004 >  num SLOAD  #1    T
| ....        SNAP   #2   [ ---- ---- 0001 ]
| 0005 >  num EQ     0004  0004
| 0006 }  tru HSTORE 0002  true
| ....        SNAP   #3   [ ---- ---- 0001 true ]

The side trace for the third snapshot emits the following IRs:

| 0001    tab TNEW   #0    #0
| 0002    p32 NEWREF 0001  "key"
| 0003    fal HSTORE 0002  false
| 0004    p32 NEWREF 0001  "key"
| 0005    tru HSTORE 0004  true

As we can see, `NEWREF` is emitted twice. This is a violation of its
semantics, so the second store isn't noticeable.

This patch prevents the second emitting of IR NEWREF by checking the last
one emitted NEWREF IR. There is no need to check NEWREFs beyond since
it guarantees the snapshot is taken after it, because it may cause table
rehashing, so all prior results are invalidated.

Sergey Kaplun:
* added the description and the test for the problem

Resolves tarantool/tarantool#7937
Part of tarantool/tarantool#9145

Reviewed-by: Maxim Kokryashkin <[email protected]>
Reviewed-by: Sergey Bronnikov <[email protected]>
Signed-off-by: Igor Munkin <[email protected]>
(cherry picked from commit 005e8ce)
igormunkin pushed a commit that referenced this pull request Jan 29, 2024
Thanks to Sergey Kaplun and Peter Cawley.

(cherry-picked from commit 1761fd2)

Assume we have the parent trace with the following IRs:

| 0001 }  tab TNEW   #0    #0
| 0002 }  p32 NEWREF 0001  "key"
| 0003 }  fal HSTORE 0002  false
| ....        SNAP   #1   [ ---- ---- 0001 ---- ]
| 0004 >  num SLOAD  #1    T
| ....        SNAP   #2   [ ---- ---- 0001 ]
| 0005 >  num EQ     0004  0004
| 0006 }  tru HSTORE 0002  true
| ....        SNAP   #3   [ ---- ---- 0001 true ]

The side trace for the third snapshot emits the following IRs:

| 0001    tab TNEW   #0    #0
| 0002    p32 NEWREF 0001  "key"
| 0003    fal HSTORE 0002  false
| 0004    p32 NEWREF 0001  "key"
| 0005    tru HSTORE 0004  true

As we can see, `NEWREF` is emitted twice. This is a violation of its
semantics, so the second store isn't noticeable.

This patch prevents the second emitting of IR NEWREF by checking the last
one emitted NEWREF IR. There is no need to check NEWREFs beyond since
it guarantees the snapshot is taken after it, because it may cause table
rehashing, so all prior results are invalidated.

Sergey Kaplun:
* added the description and the test for the problem

Resolves tarantool/tarantool#7937
Part of tarantool/tarantool#9145

Reviewed-by: Maxim Kokryashkin <[email protected]>
Reviewed-by: Sergey Bronnikov <[email protected]>
Signed-off-by: Igor Munkin <[email protected]>
ligurio added a commit that referenced this pull request Jan 17, 2025
https://github.com/tarantool/luajit/actions/runs/12831238829/job/35781196235

 =================================================================
==765154==ERROR: LeakSanitizer: detected memory leaks

Direct leak of 256 byte(s) in 1 object(s) allocated from:
    #0 0x496949 in realloc (/opt/actions-runner/_work/_temp/build-12831238829/test/tarantool-c-tests/lj-881-fix-lua-concat.c_test+0x496949)
    #1 0x4da87a in lj_mem_realloc /opt/actions-runner/_work/luajit/luajit/src/lj_gc.c:869:7
    #2 0x53f2a8 in lj_ir_growtop /opt/actions-runner/_work/luajit/luajit/src/lj_ir.c:81:23
    #3 0x53f5f1 in lj_ir_nextins /opt/actions-runner/_work/luajit/luajit/src/lj_iropt.h:34:40
    #4 0x53f5f1 in lj_ir_emit /opt/actions-runner/_work/luajit/luajit/src/lj_ir.c:118:15
    #5 0x5cc2ea in lj_record_setup /opt/actions-runner/_work/luajit/luajit/src/lj_record.c:2651:3
    #6 0x5440f0 in trace_state /opt/actions-runner/_work/luajit/luajit/src/lj_trace.c:681:7
    #7 0x58c045 in lj_vm_cpcall /opt/actions-runner/_work/_temp/build-12831238829/src/lj_vm.S:1262

SUMMARY: AddressSanitizer: 256 byte(s) leaked in 1 allocation(s).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant