Project

General

Profile

Actions

Feature #11599

open

Dump entries of hash in ObjectSpace

Added by yosiat (Yosi Attias) over 8 years ago. Updated over 8 years ago.

Status:
Open
Target version:
-
[ruby-core:71099]

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

objspace_dump_hash_entries.patch (4.8 KB) objspace_dump_hash_entries.patch Patch yosiat (Yosi Attias), 10/17/2015 12:05 PM
objspace_dump.patch (4.53 KB) objspace_dump.patch yosiat (Yosi Attias), 10/19/2015 06:49 PM

Updated by nobu (Nobuyoshi Nakada) over 8 years ago

  • Description updated (diff)

Are is_{key,value}_address necessary?

Updated by yosiat (Yosi Attias) over 8 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) over 8 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) over 8 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) over 8 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) over 8 years ago

Nobuyoshi Nakada wrote:

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.

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:

  1. If this is special const: write the inspected value of it
  2. If this is not a special const: write it's address

Updated by yosiat (Yosi Attias) over 8 years ago

Yosi Attias wrote:

Nobuyoshi Nakada wrote:

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.

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:

  1. If this is special const: write the inspected value of it
  2. 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) over 8 years ago

  • Assignee set to nobu (Nobuyoshi Nakada)

Updated by yosiat (Yosi Attias) over 8 years ago

What is the status of the patch? Is this ok? something to fix?

Updated by yosiat (Yosi Attias) over 8 years ago

Can I get a fair explanation of why this issue abandoned?

Updated by tenderlovemaking (Aaron Patterson) over 8 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) over 8 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) over 8 years ago

  • Assignee changed from nobu (Nobuyoshi Nakada) to tmm1 (Aman Karmani)
Actions

Also available in: Atom PDF

Like0
Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0Like0