Feature #11599
openDump entries of hash in ObjectSpace
Description
Hi,
This is my first c code contribution :)
I am helping developing heap-analyzer (github.com/tenderlove/heap-analyzer), and currently the dumps lacks of "type metadata" information, like:
- Hash entries - the keys and value
- Array items - the items of the array
etc.
In the included patch, I have changed the dump of hash to add entries of hash.
For example, given the next hash:
hash = {
int_key: 1,
str_key: "This is my string",
inner_hash: { b: 2 }
}
The dump result (ObjectSpace.dump(hash)) will be:
{
"address": "0x007fbc01110340",
"type": "HASH",
"class": "0x007fbc0109b400",
"size": 3,
"entries": [
{
"is_key_address": false,
"key": ":int_key",
"is_value_address": false,
"value": "1"
},
{
"is_key_address": false,
"key": ":str_key",
"is_value_address": true,
"value": "0x007fbc01110390"
},
{
"is_key_address": false,
"key": ":inner_hash",
"is_value_address": true,
"value": "0x007fbc01110368"
}
],
"references": [
"0x007fbc01110390",
"0x007fbc01110368"
],
"memsize": 232,
"flags": {
"wb_protected": true
}
}
As you can see, I have the "entries" array, where each entry contains: "is_key_address", "is_value_address" - if the key/value are special consts the inspected value will be printed in the "key"/"value" properties, other their address will be print.
Hope you will accept the patch (and I can submit another one for arrays),
Yosi.
Files
Updated by nobu (Nobuyoshi Nakada) about 9 years ago
- Description updated (diff)
Are is_{key,value}_address
necessary?
Updated by yosiat (Yosi Attias) about 9 years ago
Nobuyoshi Nakada wrote:
Are
is_{key,value}_address
necessary?
Yes, If the key/value are composite objects (not special consts, like string/hash/class) then the values of "key"/"value" should be address which is pointer to other object (like hash, string),
and to make sure there are no confusion some one did some like this:
h = {address: "0x007fbc01110368" }
Updated by nobu (Nobuyoshi Nakada) about 9 years ago
Yosi Attias wrote:
and to make sure there are no confusion some one did some like this:
h = {address: "0x007fbc01110368" }
I can't get your point here.
{"key":":a","value":"0x007f8f3c8baf88"}
seems clear enough to me.
Updated by yosiat (Yosi Attias) about 9 years ago
Nobuyoshi Nakada wrote:
Yosi Attias wrote:
and to make sure there are no confusion some one did some like this:
h = {address: "0x007fbc01110368" }
I can't get your point here.
{"key":":a","value":"0x007f8f3c8baf88"}
seems clear enough to me.
Yes, but how can you distinguish between string value which is address or address to other ruby object:
h = {a: "0x007f8f3c8baf88"}
to h = {a: {b: 1}}
Updated by nobu (Nobuyoshi Nakada) about 9 years ago
That distinction makes no sense.
Non-special-const objects (including String
) are always shown in pointer reference form.
That "0x007f8f3c8baf88"
is the content of a string but not the pointer, then it never appears with your patch.
Updated by yosiat (Yosi Attias) about 9 years ago
Nobuyoshi Nakada wrote:
That distinction makes no sense.
Non-special-const objects (includingString
) are always shown in pointer reference form.
That"0x007f8f3c8baf88"
is the content of a string but not the pointer, then it never appears with your patch.
Oh.. I understand what you are saying, I will fix that!
Just to make sure I understand your position, you say this distinction is not needed, and in case of this hash:
str_key = "Hello world".freeze
hash = {}
hash[:a] = 1
hash[str_key] = -1
You want the result to be:
{
"address": "0x007fa3d28c2dc8",
"type": "HASH",
"class": "0x007fa3d30af400",
"size": 2,
"entries": [
{
"key": ":a",
"value": "0x00000000000003"
},
{
"key": "\"Hello world\"",
"value": "0xffffffffffffffff"
}
],
"references": [
"0x007fa3d28b1230"
],
"memsize": 232,
"flags": {
"wb_protected": true
}
}
If so, I have one question:
The addresses of the values - "0x00000000000003" / "0xffffffffffffffff", dosen't exist in the dump result - because we don't dump numerics in the dump, and how I can know what is the dump result?
Or you want the value logic to be:
- If this is special const: write the inspected value of it
- If this is not a special const: write it's address
Updated by yosiat (Yosi Attias) about 9 years ago
- File objspace_dump.patch objspace_dump.patch added
Yosi Attias wrote:
Nobuyoshi Nakada wrote:
That distinction makes no sense.
Non-special-const objects (includingString
) are always shown in pointer reference form.
That"0x007f8f3c8baf88"
is the content of a string but not the pointer, then it never appears with your patch.Oh.. I understand what you are saying, I will fix that!
Just to make sure I understand your position, you say this distinction is not needed, and in case of this hash:str_key = "Hello world".freeze hash = {} hash[:a] = 1 hash[str_key] = -1
You want the result to be:
{ "address": "0x007fa3d28c2dc8", "type": "HASH", "class": "0x007fa3d30af400", "size": 2, "entries": [ { "key": ":a", "value": "0x00000000000003" }, { "key": "\"Hello world\"", "value": "0xffffffffffffffff" } ], "references": [ "0x007fa3d28b1230" ], "memsize": 232, "flags": { "wb_protected": true } }
If so, I have one question:
The addresses of the values - "0x00000000000003" / "0xffffffffffffffff", dosen't exist in the dump result - because we don't dump numerics in the dump, and how I can know what is the dump result?Or you want the value logic to be:
- If this is special const: write the inspected value of it
- If this is not a special const: write it's address
Hi,
I have added to reflect the new logic:
- If the key/value are special consts - I write the "to_s" value of them (:a => "a", 1 => "1")
- If the key/value are not special consts - they pointer reference will be written
And I have removed the "is_key_address" and "is_value_address"
Updated by yosiat (Yosi Attias) about 9 years ago
- Assignee set to nobu (Nobuyoshi Nakada)
Updated by yosiat (Yosi Attias) about 9 years ago
What is the status of the patch? Is this ok? something to fix?
Updated by yosiat (Yosi Attias) about 9 years ago
Can I get a fair explanation of why this issue abandoned?
Updated by tenderlovemaking (Aaron Patterson) almost 9 years ago
We should probably get Aman's opinion on this since he wrote the initial heap dumping code. Personally, I would like to see this feature enabled with a flag. It seems like it will significantly increase the size of the dump file, and also the dumped data will not be backwards compatible. Maybe if we add a flag to the dump
method it would be easier to take this patch?
Updated by yosiat (Yosi Attias) almost 9 years ago
Aaron Patterson wrote:
We should probably get Aman's opinion on this since he wrote the initial heap dumping code. Personally, I would like to see this feature enabled with a flag. It seems like it will significantly increase the size of the dump file, and also the dumped data will not be backwards compatible. Maybe if we add a flag to the
dump
method it would be easier to take this patch?
Thanks :)
Just throwing out - If the dump size is something that matters and we want to decrease it - I am thinking about writing in some binary format (msgpack/protobuf, with protobuf you will get backward compatible).
Updated by yosiat (Yosi Attias) almost 9 years ago
- Assignee changed from nobu (Nobuyoshi Nakada) to tmm1 (Aman Karmani)
Updated by hsbt (Hiroshi SHIBATA) 8 months ago
- Status changed from Open to Assigned