Skip Menu |

This queue is for tickets about the Inline-Struct CPAN distribution.

Report information
The Basics
Id: 101050
Status: resolved
Worked: 3.5 hours (210 min)
Priority: 0/
Queue: Inline-Struct

People
Owner: ETJ [...] cpan.org
Requestors: heinz.knutzen [...] gmail.com
Cc:
AdminCc:

Bug Information
Severity: (no value)
Broken in: (no value)
Fixed in: 0.18



Subject: Can't use hashref as parameter to "new"
Date: Sat, 20 Dec 2014 23:29:02 +0100
To: bug-Inline-Struct [...] rt.cpan.org
From: Heinz Knutzen <heinz.knutzen [...] gmail.com>
The manual page of Inline::Struct states: "new ... If you provide values, they should be appropriate for the field type, and in the same order as they are defined in the struct." When looking at the source code, we see that the first value is handled specially if it is an array or hash ref. In this case it is used to initialize multiple fields. 1. problem: If the initial value is a hash ref and it is passed to "new" inside a hashref, the value can't be retrieved later. 2. problem: There is a conceptual problem, if the first field is of type SV *. If the first parameter to "new" is a hash ref, it can't be decided, if the value should be used to initialize the first field or if it should be handled specially. Test case: ============================================ use strict; use warnings; use Test::More; use Inline C => <<'END', structs => 1, force_build => 1, clean_after_build => 0; struct Foo { SV *hash; }; END my $HASH = { a => { b => 'c' } }; my $o = Inline::Struct::Foo->new({hash => $HASH}); is_deeply $o->hash, $HASH, "hashref retrieved"; my $p = Inline::Struct::Foo->new($HASH); is_deeply $p->hash, $HASH, "hashref retrieved"; done_testing; ============================================ This fails for me with: not ok 1 - hashref retrieved # Failed test 'hashref retrieved' # at svmember.t line 13. # Structures begin differing at: # $got = '157264960' # $expected = HASH(0x95d0da8) No such field 'a' in struct Foo # Tests were run but no plan was declared and done_testing() was not seen. Used versions: - Inline-Struct-0.16 - Perl v5.20.1
You're quite right - adding SV * as a possible type has made the args ambiguous. Additionally, the benchmark showed that setting struct values is slower than using Perl hashes. I believe this is because of the set-mode of methods doing the work of returning the object, with the associated refcount-twiddling, and also runtime polymorphism (being able to call the same method with different args and getting different behaviour). I therefore propose (in the absence of grep.cpan.me showing any CPAN modules that actually use Inline::Struct) changing the API in these ways: * new will take only a list of appropriately-type data; the hash- and array-ref args will go; * removing the object-returning feature of the set-mode of accessors; in any case a better pattern for multiple setting is eg "map { $s->$_($data{$_}) } keys %data". I am also contemplating doing away with the runtime polymorphism entirely by just having methods get_membername and set_membername; this will speed up both setting and getting by reducing the steps needed. Obviously the $VERSION would jump to 1.0, since the API will have changed. As the only known user of this module, what are your thoughts?
On further checking, you're quite right that while the code supports new's argument being the two types of ref, the doc does not. Therefore the first point above is in fact already the documented behaviour, as reflected in the new 0.17 (with a test). The other points remain, however, and your thoughts are still welcome.
To address my second point, I made a branch off 0.18 called "no-objreturn", to compare performance. Remarkably, deleting the relevant code actually made performance slightly worse.
To address my third point, I made another branch off 0.18 called "no-polymorphism". The writes got a little faster, but the reads got a little slower; I speculate this is due to CPU caching. I'm going to mark this resolved, but please feel free to offer your thoughts, since I am still interested.
Subject: Re: [rt.cpan.org #101050] Can't use hashref as parameter to "new"
Date: Thu, 01 Jan 2015 23:03:54 +0100
To: bug-Inline-Struct [...] rt.cpan.org
From: Heinz Knutzen <heinz.knutzen [...] gmail.com>
Feel free to remove
  1. the runtime polymorphism;
    I can live without this feature.
  2. the object-returning feature;
    I would never use this.
Currently I don't use this module because the memory usage is worse than for pure Perl hashes.
I think memory and runtime is wasted by global hash %_map_ that is used internally by Inline::Struct.
For each new struct, a new hash entry is added to %_map_ and the value is another anonymous hash with keys "REFCNT" and "FREE".

Inline::CPP has some good examples for classes and accessors.
It seems they get along without something similar to %_map_.
I have rewritten your benchmark script to use Inline::CPP instead of Inline::Struct and got these encouraging results:

$ perl inline-cpp-benchmark.pl
Faster type          % faster
CPP dnum read        15%
CPP dnum write       26%
CPP inum read        35%
CPP inum write       27%
CPP str read         24%
CPP str write        20%
Memory usage
10000 x bless [ 7, "string" ], "main": 6200
10000 x Foo->new: 5812
100000 x bless [ 7, "string" ], "main": 21388
100000 x Foo->new: 17468
1000000 x bless [ 7, "string" ], "main": 173184
1000000 x Foo->new: 133916

Find my inline-cpp-benchmark.pl as attachment.

Message body is not shown because sender requested not to inline it.

FYI I've just released 0.19 with some of the improvement we discussed but I didn't release at the time, not sure why. I updated the two branches mentioned above. I will be pleased to consider any changes you feel like submitting!