> On Nov. 21, 2012, 12:52 a.m., Ben Mahler wrote: > > src/common/resources.hpp, line 281 > > <https://reviews.apache.org/r/8145/diff/1/?file=222293#file222293line281> > > > > This vector of pairs looks a bit rough for a caller to use. > > > > 1. Any reason to not use uint16_t? It is the true size of port numbers, > > using uint64_t might require some error handling on the caller. > > > > 2. Could we just use a std::set or hashset of the ports available? > > > > This might be a bit high on the memory front: > > maximum size = (65536 ports * 2 bytes / 1024) = 128KB + overhead. > > But expected case would definitely be no more than a quarter of that. > > > > I think we should do this for simplicity. > > > > Longer term, if performance / memory warrants it, we could have a > > Ranges abstraction that only stores the ranges and provides set membership > > checks?
I used uint64_t because thats what 'Range' takes in the protobuf. But I guess I can just down convert 64bit numbers to 32bit when getting ports. Regarding the 2nd point, I did consider using a list of ports, but disregarded it because of its memory inefficiency. All our operations on ranges just work on ranges without converting them to a flat list. But, I agree on providing set membership abstractions for ranges. We can probably extend our 'ranges' namespace in values.cpp. Not part of this review though. - Vinod ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/8145/#review13654 ----------------------------------------------------------- On Nov. 20, 2012, 11:53 p.m., Vinod Kone wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/8145/ > ----------------------------------------------------------- > > (Updated Nov. 20, 2012, 11:53 p.m.) > > > Review request for mesos, Benjamin Hindman and Ben Mahler. > > > Description > ------- > > Added helpers to extract specific resources: cpus,mem, disk and ports. > > I wanted these as part of a fix I am going to make to the slave, to properly > deal with resources specified via command line. > > Hopefully, once we make the resources first class, we don't need these > helpers? > > > Diffs > ----- > > src/common/resources.hpp 5237b6031919aaed2d742bc095e537e83bf9f3fd > src/tests/resources_tests.cpp 83e93482a6800724d51aeae91e57d2a2b52b37b6 > > Diff: https://reviews.apache.org/r/8145/diff/ > > > Testing > ------- > > make check > > > Thanks, > > Vinod Kone > >
