Consensys / linea-arithmetization

17 stars 18 forks source link

limit the size of moduleCalls in ImcFragment.java #807

Open ahamlat opened 1 week ago

ahamlat commented 1 week ago

This issue is similar to https://github.com/Consensys/linea-arithmetization/issues/801. We're tracking all object allocations that are referenced from TraceSection, as we may have hundred of thousands instances of TraceSection. In the example below, elementData consumes 56 bytes shallow size because the size by default is 10, and in this case elementData inside the arrayList consumes 56 bytes (see https://github.com/Consensys/linea-arithmetization/issues/801).

image

Reducing the size to 1 in this case would reduce the (shallow) size of elementData from 56 to 24

**** without size *****
Class: ArrayList
# WARNING: Unable to get Instrumentation. Dynamic Attach failed. You may add this JAR as -javaagent manually, or supply -Djdk.attach.allowAttachSelf
# WARNING: Unable to attach Serviceability Agent. You can try again with escalated privileges. Two options: a) use -Djol.tryWithSudo=true to try with sudo; b) echo 0 | sudo tee /proc/sys/kernel/yama/ptrace_scope
Retained size: java.util.ArrayList@73f792cfd object externals:
          ADDRESS       SIZE TYPE                PATH                           VALUE
        70fd204e8         24 java.util.ArrayList                                (object)
        70fd20500        192 (something else)    (somewhere else)               (something else)
        70fd205c0         56 [Ljava.lang.Object; .elementData                   [1, null, null, null, null, null, null, null, null, null]
        70fd205f8    1028568 (something else)    (somewhere else)               (something else)
        70fe1b7d0         16 java.lang.Integer   .elementData[0]                1

Addresses are stable after 1 tries.

**** size = 1 *****
Class: ArrayList
Retained size: java.util.ArrayList@4c583ecfd object externals:
          ADDRESS       SIZE TYPE                PATH                           VALUE
        70f2559f8         24 java.util.ArrayList                                (object)
        70f255a10         24 [Ljava.lang.Object; .elementData                   [2]
        70f255a28   12344760 (something else)    (somewhere else)               (something else)
        70fe1b7e0         16 java.lang.Integer   .elementData[0]                2

Addresses are stable after 1 tries.

Fixing the size works only if we know the target size, and the array list is not resized after initialization.

letypequividelespoubelles commented 1 week ago

I put in https://github.com/Consensys/linea-arithmetization/pull/748/commits/b99310d7d6aae8a2886141d37f0b4ebef9595a9e the default size to 5 as it's the max for our arithmetization. Is it usefull to have something even better ?

ahamlat commented 1 week ago

5 is already a good one, because it will reduce from the shallow size of the underlying array from 56 to 40. Do you know if the size can go beyond 5 ?

letypequividelespoubelles commented 1 week ago

No it can't, it's the max. It'll always be between 1 and 5

letypequividelespoubelles commented 1 week ago

Should we close this as completed or there will be an impact to provide the exact nb of moduleCall ?

ahamlat commented 1 week ago

I think for now, we can make it simple, i.e keep the size configured to 5, and do another profiling work after le PR is merged.