{"action":"created","issue":{"url":"https://api.github.com/repos/eclipse/openj9/issues/1032","repository_url":"https://api.github.com/repos/eclipse/openj9","labels_url":"https://api.github.com/repos/eclipse/openj9/issues/1032/labels{/name}","comments_url":"https://api.github.com/repos/eclipse/openj9/issues/1032/comments","events_url":"https://api.github.com/repos/eclipse/openj9/issues/1032/events","html_url":"https://github.com/eclipse/openj9/issues/1032","id":291944010,"number":1032,"title":"Potential footprint issue with the stackmap frames during runtime verification","user":{"login":"ChengJin01","id":18036229,"avatar_url":"https://avatars3.githubusercontent.com/u/18036229?v=4","gravatar_id":"","url":"https://api.github.com/users/ChengJin01","html_url":"https://github.com/ChengJin01","followers_url":"https://api.github.com/users/ChengJin01/followers","following_url":"https://api.github.com/users/ChengJin01/following{/other_user}","gists_url":"https://api.github.com/users/ChengJin01/gists{/gist_id}","starred_url":"https://api.github.com/users/ChengJin01/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/ChengJin01/subscriptions","organizations_url":"https://api.github.com/users/ChengJin01/orgs","repos_url":"https://api.github.com/users/ChengJin01/repos","events_url":"https://api.github.com/users/ChengJin01/events{/privacy}","received_events_url":"https://api.github.com/users/ChengJin01/received_events","type":"User","site_admin":false},"labels":[{"id":697962110,"url":"https://api.github.com/repos/eclipse/openj9/labels/comp:vm","name":"comp:vm","color":"1d76db","default":false},{"id":676460807,"url":"https://api.github.com/repos/eclipse/openj9/labels/enhancement","name":"enhancement","color":"84b6eb","default":true},{"id":778340830,"url":"https://api.github.com/repos/eclipse/openj9/labels/footprint","name":"footprint","color":"5319e7","default":false}],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":72,"created_at":"2018-01-26T15:51:15Z","updated_at":"2018-05-03T23:41:05Z","closed_at":null,"author_association":"CONTRIBUTOR","body":"In the existing implementation, all raw data in stackmap tables (if exists) are always decompressed to\r\na new stackmap stable with more readable data for later verification. However, it might occupy a bigger memory space as compared to the original stackmap table. which in turn seriously impacts the footprint at startup/runtime.\r\n\r\nThis issue is intended to investigate how much memory space it wastes in term of the decompressed stackmap table in most cases.  If it wastes far more than anticipation, we need to come up with a solution which should be feasible to improve the footprint.\r\n\r\n1) what steps to do to check the wasted memory space?\r\n   [1]check via DDR\r\n   [2]check via -verbose:stackmapload\r\n   [3] Another way is to directly check the occupied memory by adding printing message when allocating memory  for the stackmap in the following code:\r\n\r\n    /runtime/bcverify/bcverify.c\r\n   ...\r\n   static UDATA maximalMemoryStackmap = 0;\r\n   ...\r\n   j9bcv_verifyBytecodes (J9PortLibrary * portLib, J9Class * clazz, J9ROMClass * romClass,\r\n\t\t   J9BytecodeVerificationData * verifyData)\r\n{\r\n  ...\r\n  \t\t\tverifyData->stackSize = (J9_MAX_STACK_FROM_ROM_METHOD(romMethod)\r\n\t\t\t\t\t\t\t\t\t+ J9_ARG_COUNT_FROM_ROM_METHOD(romMethod)\r\n\t\t\t\t\t\t\t\t\t+ J9_TEMP_COUNT_FROM_ROM_METHOD(romMethod)\r\n\t\t\t\t\t\t\t\t\t+ BCV_TARGET_STACK_HEADER_UDATA_SIZE\r\n\t\t\t\t\t\t\t\t\t+ BCV_STACK_OVERFLOW_BUFFER_UDATA_SIZE) * sizeof(UDATA);\r\n\t\t\t\t...\r\n  \t\t\tstackMapsSize = (verifyData->stackSize) * (verifyData->stackMapsCount);\r\n\t\t\tALLOC_BUFFER(verifyData->stackMaps, stackMapsSize);\r\n\t\t\t\r\n\t\t\tif (maximalMemoryStackmap < stackMapsSize) {\r\n\t\t\t    maximalMemoryStackmap = stackMapsSize;\r\n\t\t\t    printf(\"\\nstackMapsSize = %u\\n\", maximalMemoryStackmap);\r\n\t\t\t}\r\n\t\t\t...\r\nGiven that the memory for the stackmap is reused for all methods(to be verified) and gets reallocated only when the new requested size is bigger than the allocated memory, we only need to check the maximal value of stackMapsSize in such case.\r\n\r\n2) which one of the following solutions is the preferable if the issue exists and matters to improve the footprint?\r\n\r\n[1] Solution 1:\r\n   Given that rom classes are already stored in the shared cache (including lineNumber, debugInfo, etc),\r\n   we can take advantage of the shared cache to store the new stackmap table. The intention here is that the StackMapTables could be stored out of line in the sharedclasses cache and then the memory for them could be disclaimed after the VM exists startup_phase.\r\n\r\n   The benefit of doing this is obvious:\r\n   1) there is literally no memory space spent on the new stackmap table.\r\n   2) it would accelerate the runtime verification except the first-time startup (decompressed only once)\r\n\r\n[2]Solution 2:\r\n    Another alternative is an one-round verification which means it decodes one specific frame from the raw stackmap table only when checking the consistency of the stackmap frame during verification.\r\n   \r\n   Theoretically, the total time spent on the verification in this solution should be equivalent of an one-round traverses on the original stackmap table, in which case it should spend far less time than the original implementation.  (The original implementation needs two-round traverses on the stackmap table: first of all, decompress the stackmap table (also involves the reallocation of memory; secondly), go through the new stackmap table to do verification.)\r\n\r\n   Meanwhile, there is actually only one stackmap frame of memory that should be allocated/reusd during the verification.\r\n\r\n   The downside of solution 2 as compared to the solution 1 include:\r\n   1) It literally needs to decode the stackmap table every time when doing verification (there is no time spent on decompression in solution [1] except the first-time startup) \r\n   2) Memory still needs to be allocated for a stackmap frame while there is no memory allocated in solution [1] (stored in the shared cache).\r\n   3) It is unclear for now how many changes are involved in the detailed error message framework followed by such adjustment.\r\n"},"comment":{"url":"https://api.github.com/repos/eclipse/openj9/issues/comments/386468728","html_url":"https://github.com/eclipse/openj9/issues/1032#issuecomment-386468728","issue_url":"https://api.github.com/repos/eclipse/openj9/issues/1032","id":386468728,"user":{"login":"pshipton","id":18122812,"avatar_url":"https://avatars1.githubusercontent.com/u/18122812?v=4","gravatar_id":"","url":"https://api.github.com/users/pshipton","html_url":"https://github.com/pshipton","followers_url":"https://api.github.com/users/pshipton/followers","following_url":"https://api.github.com/users/pshipton/following{/other_user}","gists_url":"https://api.github.com/users/pshipton/gists{/gist_id}","starred_url":"https://api.github.com/users/pshipton/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/pshipton/subscriptions","organizations_url":"https://api.github.com/users/pshipton/orgs","repos_url":"https://api.github.com/users/pshipton/repos","events_url":"https://api.github.com/users/pshipton/events{/privacy}","received_events_url":"https://api.github.com/users/pshipton/received_events","type":"User","site_admin":false},"created_at":"2018-05-03T23:41:05Z","updated_at":"2018-05-03T23:41:05Z","author_association":"CONTRIBUTOR","body":"- not sure there is enough stack map data to make any significant difference in the footprint. It would be worth collecting some stats\r\n- seems like cold runs without the stack map changes could be optimized to disclaim a bigger portion of the metadata\r\n- not sure about the duplicated methods comment, are you using the hash to de-duplicate stack map data by itself? If so, how much duplicate data is there? ROM class de-duplication will work no matter how the stack map data is found.\r\n- an SRP can be encoded in the ROM class in the place where the stack map size is normally stored. It should be more efficient, for both footprint and time, to use a SRP than a hashtable"}}
{"actor":{"display_login":"pshipton"}}